Suggested code - handling URL redirects

General JUCE discussion
1

As discussed recently, there's been a change to the URL code so that it follows redirects (see http://www.juce.com/forum/topic/how-get-url-follow-redirects?page=1#comment-307967).

The altered code attached below allows arbitrary setting of the number of redirects to be followed. It's implemented and tested for Windows and OS X. It should be OK for iOS, but I'm not set up to test that.

Jules - it'd be great if you could look at including this.

I haven't implemented anything for Android (again I'm not set up for Android development) - however I think this can be achieved by calling HttpURLConnection::setFollowRedirects(false) and looping as per the Windows code. It'd be great if someone could have a go at this (see createHTTPStream in JuceAppActivity.java).

 

EDIT 1: got myself mucked up between platforms and used the wrong Win32 file, now fixed. The for loop should be as follows:

​for (int numRedirects = numRedirectsToFollow; numRedirects >= 0; --numRedirects)

EDIT 2: I'm trying to get set up to try this on Android and have a fair way to go, but found that I introduced a parenthesis error in juce_Android_Network.cpp - this is now fixed. I'll update again as I make more progress.

EDIT 3: It was hard work, but I've now sorted out the Android code. While I was at it I normalised the connection time out, avoided crashes with null statusCode[] or postData[] and added handling for processing of request headers.

EDIT 4: Mac code now confirmed to work OK on iOS also. That just leaves linux now...

EDIT 5: I've had a go at the Linux code and it now seems to handle redirects OK (note that the current JUCE code has a bug where the redirect counter is reset and therefore no limit is enforced).

EDIT 6: OK, so I've created some unit tests for this  (see last 6 files). Unsurprisingly this threw up a few more inconsistencies which I've now addressed for all platforms except Linux.

EDIT 7: Linux code has now been re-included (note that HTTPS is not supported).

2

I just realised that linux will need some love too. Any volunteers?

3

Android code now sorted, see edit 3 in first post.

4

Linux code now added, see edit 5 in first post.

If I get time later in the week I might write some unit tests to ensure that functionality is consistent across all platforms. I've ironed out a few, but it's probably worth being pedantic...

5

Note that I've withdrawn the linux code while I hammer out a problem with relative redirect paths.

6

The Linux code has now been sorted and re-attached to the first post. 

I think I've taken the URL class as far as I can for now. Overall, there is improved consistency between all the platforms and of course there is now more control over redirects. The unit tests are pretty handy, but they do show up the following issues:

  • on rare occasions on the Mac (but not iOS) didFinishLoading is scheduled before didReceiveResponse 
    <ul>
	<li><span>this only ever occurs during the redirect chain unit test, and then only sporadically</span></li>
	<li><span>this can only happen if an error occurs before a response could be created, but I am unable to trap an error</span></li>
	<li><span>the result is that a null statusCode is returned (i.e. the code doesn't bomb)</span></li>
</ul>
</li>
<li><span>on rare occasions, the Linux code bombs out</span>
<ul>
	<li><span>I found and fixed a couple of things in the original code</span></li>
	<li><span>however I've been unable to trap the remaining edge cases as they're too sporadic</span></li>
	<li><span>note that I'm not convinced of the stability of the VM environment I'm running Linux in</span></li>
</ul>
</li>

I've spent too many long nights & looking at this stuff, so it'd be great if someone with fresher eyes could take a look - please :)

7

Well it's been a couple of weeks and no-one else in the community has offered to cross-check any of this so I'm still busy talking to myself :)

Jules - is this on your radar at all?

8

Yes, it's on my radar - I've got the thread open as a tab in my browser, so will look at it soon!

9

Ok, cool!

10

FYI I've finally committed this - please let me know if it works!

11

Yep looks good -the unit tests all pass on Win, OSX, iOS, Android & Linux.

Thanks Jules!