Showing posts with label objective-c. Show all posts
Showing posts with label objective-c. Show all posts

Tuesday, 14 September 2021

Frustrating loop with NSURLSession when implementing URLSession: task: willPerformHTTPRedirection: newRequest: completionHandler:

This isn't really a question, I think I've solved the problem. But I think I've seen this in the past and solved it then. So I'm posting this here in case anyone else - most likely future me - has this same problem and wants to save some time.

Context: I'm working on the next version of my crawling engine - it moves to NSURLSession rather than NSURLConnection**. Therefore (with the connection stuff at least) it's a ground-up rewrite. 

Here's the problem I hit this morning. A Youtube link is redirecting from a shortened link to a longer one. But some unexpected things are happening. 


My URLSession: task: willPerformHTTPRedirection: newRequest: completionHandler:   is being called and at first I simply put in the minimum code needed to simply continue with the redirection:
if(completionHandler){
    completionHandler(request)
}

According to example code found in various places, that should work. The documentation suggests that this is fine: "completionHandler: A block that your handler should call with either the value of the request parameter, a modified URL request object, or NULL to refuse the redirect and return the body of the redirect response."

As you can see from the screenshot above, our url is being redirected over and over, each time with some garbage appended to the querystring. 

Integrity and Scrutiny handle this fine. (Though at this point they are using the old NSURLConnection.) However, they have a lot more code in the redirect delegate method than my new one. Notably, they make a mutable copy of the new request and make some changes. Why do they need to do that?

I've a funny feeling that I've seen this problem before. Indeed, adding code to make a clone of the new request and modify it is what cures this problem.

It's not enough to simply clone and return the new request.  This is the line that makes the difference:

[clonedRequest  setValue: [[request URL] host] forHTTPHeaderField: @"Host"];

(There's more to it than that, but there are reasons why my code isn't open source! Also, I use Objective C, apologies if you're a Swift person*).

In short, when we create the original request, we set the Host field. In the past I have found this to be necessary in certain cases. In fact, the documentation says that the Host field must be present with http 1.1 and a server may return a 4xx status if it's not present and correct.

If we capture the header fields of the proposed new request in our delegate method, the original Host field appears unchanged. Therefore, it no longer matches the actual host of the updated request url. Here, the url is being redirected to https://www.youtube.com/watch?v=xyz, but the Host field remains "youtu.be"

As I mentioned, I've written the fix into Integrity and Scrutiny in the dim and distant past, presumably because I have spent time on this problem before. 

I'm guessing that this isn't a problem if you don't explicitly set the Host field yourself in your original request, but if you don't then you may find other problems pop up occasionally. 

If future me is reading this after starting another ground-up project and running into the same problem: you're welcome.



** The important delegate methods of NSURLSessionTask are very similar to the old connection ones. Because I'm seeing similar behaviour with both, I do believe that beneath the skin, NSURLSession and NSURLConnection are just the same. 

* I'm aware that many folks are using Swift today, and it's getting harder to find examples and problem fixes written in Objective C. I'm expecting that (as with java-cocoa) Apple will eventually remove support. But I won't switch and I'm grateful for Objective C support as long as it lasts.

Wednesday, 6 June 2018

How did that ever work?

Don't judge the code - this is a tool that was written many many years ago, and it's only a simple thing for personal use. So it was only ever developed to the "it just about works" standard and the project has been copied from computer to computer ever since, receiving the minimum updates to keep it running.

With the beta of 10.14 'Mojave' installed on a mac (which sounds enough like 'Mojito' to raise my pulse) unsurprisingly I started to notice a few things not working as before.

I love finding and fixing problems, so the regular round of fixes with each release of OSX / MacOS  is no hardship. It's particularly fun when you have a "how did that ever work?" moment.

NSArray *pages = [fileManager directoryContentsAtPath:pageLocation];   // NB directoryContentsAtPath: was apparently deprecated in 10.5

if([pages count]==0){return;}
for (c=0;c<[pages count];c++){
// foreach page in the pages directory
thisPage = (NSString *)[pages objectAtIndex:c];
if([[thisPage substringToIndex:1] isEqualToString:@"."]==NO){
// do stuff, ignoring hidden files
[collection addObject:thisPage];
}

}

The resulting list is displayed in a tableview and has always appeared in alphabetical order.

So not only is directoryContentsAtPath: still apparently working after being deprecated such a long time ago, apparently it used to return the directory listing sorted in alphabetical order, and no longer does.

It was easy to add [collection sortUsingSelector:@selector(caseInsensitiveCompare:)];
to restore the list to alphabetical order (collection being an NSMutableArray containing NSStrings) but I'm just surprised  that it wasn't necessary before.

The documentation for directoryContentsAtPath: doesn't mention that the return array is sorted, so it should never have been taken for granted. But hey, if something works the way you want it, you don't always think any further.

To bring this up to scratch, the suggested alternative to directoryContentsAtPath: is 
contentsOfDirectoryAtPath:Error:  so getting rid of that warning is really easy, just declare an NSError object and pass it in. And then report the NSError's 'localizeddescription' if it contains a non-null value. Or simply pass nil as the error: parameter if you feel lazy or don't care about the success of the operation.