GNOME Bugzilla – Bug 604383
libsoup does not handle recursive or infinite redirections (patch attached)
Last modified: 2010-06-08 15:21:19 UTC
Created attachment 149598 [details] [review] Patch which makes libsoup to count redirections libsoup does not handle recursive or infinite redirections, so if one page sends a Location header refering to itself (as an example), libsoup will be requeuing the message over and over. The patch I am attaching add a new field to SoupMessagePrivate, and two methods, declared in soup-message-private.h, to manage it. SoupSession will check each time a message is redirected how many redirections had been previously made, and if there have been more than 20, the message is cancelled.
Comment on attachment 149598 [details] [review] Patch which makes libsoup to count redirections yes, we've needed this for a while. is there any reason you picked "20"? do we know what other browsers do? for the error code name, I think "SOUP_STATUS_TOO_MANY_REDIRECTS" is better, and also, you need to add an entry to the reason_phrases[] array in soup-status.c. (Again, probably "Too many redirects".) Also, rather than putting this in SoupMessagePrivate, it should be in SoupMessageQueueItem, and then soup-session.c can just check/increment the count itself. You can change queue_message() to pass "item" rather than "session" for the user_data, and then use item->session to get the session in redirect_handler(). Oh, and also, we should add a test to tests/redirect-test.c. (I'll probably do this during the hackfest if you don't get to it.)
(In reply to comment #1) > (From update of attachment 149598 [details] [review]) > yes, we've needed this for a while. > > is there any reason you picked "20"? do we know what other browsers do? I tested it in chrome, firefox, safari, opera and wget. They all use "20" as the limit of the allowed redirections.
Created attachment 149788 [details] [review] Patch which makes libsoup to count redirections This patch takes into account what Dan said in comment 1. (redirection count stored in SoupMessageQueueItem, SOUP_STATUS_TOO_MANY_REDIRECTS, item instead of session passed to queue_message)
Created attachment 149789 [details] [review] New test: Recursive/infinite redirection This patch adds a new test to tests/redirect-test.c to test that this bug is fixed. As storing the whole 20 redirections in the tests array was in my opinion not a good idea (the struct of the elements of the array may have been changed so that the TestRequest array lenght was set to a value of 22), i decided to change the TestRequest structure. I added two new fields to the structure: repeat and counter. Repeat stores the number of times a redirection may be received/sent, counter is used each time the test is ran to store the number of times a redirection has been received/sent.
very belatedly pushed with minor fixes. thanks