[Pidgin] #2953: libpurple parse_redirect() redirects on Content-Location header, instead of only Location

[Pidgin]

#2953: libpurple parse_redirect() redirects on Content-Location header, instead of
only Location
--------------------------------------------------------+-------------------
 Reporter:  jeff                                        |       Owner:  jeff 
     Type:  defect                                      |      Status:  new  
 Priority:  minor                                       |   Milestone:       
Component:  libpurple                                   |     Version:  2.1.1
 Keywords:  msimprpl libpurple redirect parse_redirect  |     Pending:  0    
--------------------------------------------------------+-------------------
 from libpurple/util.c
 {{{
 static gboolean
 parse_redirect(const char *data, size_t data_len, gint sock,
                            PurpleUtilFetchUrlData *gfud)
 {
         gchar *s;
         gchar *new_url, *temp_url, *end;
         gboolean full;
         int len;

         if ((s = g_strstr_len(data, data_len, "Location: ")) == NULL)
                 /* We're not being redirected */
                 return FALSE;
 }}}

 Th g_strstr_len() call matches not only the Location header, but also
 Content-Location. From RFC2616:

 > The Content-Location value is not a replacement for the original
 requested URI; it is only a statement of the location of the resource
 corresponding to this particular entity at the time of the request.

 and

 > Note: The Content-Location header field (section 14.14) differs from
 Location in that the Content-Location identifies the original location of
 the entity enclosed in the request.

 The bug in libpurple causes purple_util_fetch_url() to redirect infinitely
 (until the limit is reached) upon fetching
 http://im.myspace.com/nsis/currentversion.txt.

 Fix should be simple - just make sure only the Location header is matched,
 not Content-Location.

 Transcript from #pidgin:
 {{{
 (11:05:21 PM) KingAnt: jeff2: Yes, I believe that is intended
 (11:05:30 PM) emeriste: There is not even an official Yahoo
 application for Linux users to chat on Yahoo.
 KingAnt KingWalterI1
 (11:06:39 PM) jeff2: KingAnt: any way to prevent it? some of myspace's
 servers always send a content-location header, so libpurple loops
 indefinitely until it hits the maximum # of redirects...
 (11:07:33 PM) jeff2: even though the content is right there in the http
 response
 (11:07:50 PM) KingAnt: jeff2: I don't know of a way to prevent that
 (11:07:59 PM) KingAnt: Is that a violation of the http spec?
 (11:08:48 PM) KingAnt: jeff2: You would probably need to add a
 function or something that allows you to disable redirects for a given
 PurpleUtilFetchUrlData
 KingAnt KingWalterI1
 (11:11:34 PM) KingAnt: jeff2: Oh, er, is this a bug in our http parser?
 (11:11:53 PM) jeff2: KingAnt: probably :) the content-location header
 is always the same as the get url. think it would be okay to add a
 check to not redirect if the content-location header is the url
 originally requested?
 (11:12:21 PM) jeff2: KingAnt: I thought it might be, but it appears to
 be intentional that any header containing 'location' is parsed
 (11:12:22 PM) KingAnt: parse_redirect() in libpurple/util.c searchs
 for the "Location:" header, which accidentally matches the
 Content-Location header
 (11:12:49 PM) KingAnt: Why does it appear to be intentional?
 (11:13:07 PM) jeff2: KingAnt: oh, you're right. that's not intentional
 (11:13:14 PM) jeff2: "The Content-Location value is not a replacement
 for the original    requested URI; it is only a statement of the
 location of the resource    corresponding to this particular entity at
 the time of the request."
 (11:13:23 PM) KingAnt: Can we just change g_strstr_len(data, data_len,
 "Location: ") to g_strstr_len(data, data_len, "\r\nLocation: ")  ?
 (11:14:24 PM) jeff2: KingAnt: looks like that would work
 (11:14:26 PM) KingAnt: Or maybe just "\n" to guard against servers
 that only use \n instead of both \r\n
 (11:14:41 PM) KingAnt: Cool
 (11:14:54 PM) jeff2: KingAnt: yeah, that's a good point
 (11:14:54 PM) KingAnt: Well try that and if it fixes the problem and
 doesn't seem to break anything then I say go for it
 (11:15:07 PM) jeff2: KingAnt: hopefully no server sends just \r's :-)
 (11:15:09 PM) ecoffey left the room (quit: Remote closed the connection).
 (11:15:34 PM) jeff2: KingAnt: ok, will do. my installation is
 currently broken but i'll file a ticket with this conversation so this
 info isn't lost
 (11:15:48 PM) KingAnt: Well a web server probably wouldn't, but I
 could see maybe a cgi script doing so
 (11:16:32 PM) jeff2: possibly
 (11:17:15 PM) jeff2: strstr("Location") && !strstr("Content-Location")
 might be safer, at the expense of matching any other Location headers
 }}}

-- 
Ticket URL: <http://developer.pidgin.im/ticket/2953>
Pidgin <http://pidgin.im>
Pidgin