From 6bb15c344d568fea2a56ff2288ddd44ef1735565 Mon Sep 17 00:00:00 2001 From: Richard Frith-Macdonald Date: Thu, 16 Apr 2020 09:39:35 +0100 Subject: [PATCH 1/6] Fix use of OBJC_RUNTIME_LIB before it was initialised. Thanks to johannes@brakensiek.info for spotting this. --- configure | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure b/configure index bd23b27e5..36a45d1b0 100755 --- a/configure +++ b/configure @@ -2727,6 +2727,7 @@ fi if test -z "$LIBRARY_COMBO"; then LIBRARY_COMBO=`gnustep-config --variable=LIBRARY_COMBO 2>&5` fi +OBJC_RUNTIME_LIB=`echo $LIBRARY_COMBO | tr '-' ' ' | awk '{print $1}'` if test "$OBJC_RUNTIME_LIB" = "ng" -o "$OBJC_RUNTIME_LIB" = "apple"; then nonfragile=yes @@ -6003,7 +6004,6 @@ esac #-------------------------------------------------------------------- # Set Apple/Darwin/OSX/NeXT information for other tests #-------------------------------------------------------------------- -OBJC_RUNTIME_LIB=`echo $LIBRARY_COMBO | tr '-' ' ' | awk '{print $1}'` { $as_echo "$as_me:${as_lineno-$LINENO}: checking the Objective-C runtime" >&5 $as_echo_n "checking the Objective-C runtime... " >&6; } if test "$OBJC_RUNTIME_LIB" = "nx" -o "$OBJC_RUNTIME_LIB" = "apple"; then diff --git a/configure.ac b/configure.ac index d597f5cbd..6fb0d1d6e 100644 --- a/configure.ac +++ b/configure.ac @@ -89,6 +89,7 @@ fi if test -z "$LIBRARY_COMBO"; then LIBRARY_COMBO=`gnustep-config --variable=LIBRARY_COMBO 2>&5` fi +OBJC_RUNTIME_LIB=`echo $LIBRARY_COMBO | tr '-' ' ' | awk '{print $1}'` if test "$OBJC_RUNTIME_LIB" = "ng" -o "$OBJC_RUNTIME_LIB" = "apple"; then nonfragile=yes @@ -1254,7 +1255,6 @@ esac #-------------------------------------------------------------------- # Set Apple/Darwin/OSX/NeXT information for other tests #-------------------------------------------------------------------- -OBJC_RUNTIME_LIB=`echo $LIBRARY_COMBO | tr '-' ' ' | awk '{print $1}'` AC_MSG_CHECKING(the Objective-C runtime) if test "$OBJC_RUNTIME_LIB" = "nx" -o "$OBJC_RUNTIME_LIB" = "apple"; then AC_MSG_RESULT(NeXT) From d048dfbea540ffbc466e5dbd7091fd50cc48fafd Mon Sep 17 00:00:00 2001 From: Richard Frith-Macdonald Date: Thu, 16 Apr 2020 09:39:35 +0100 Subject: [PATCH 2/6] Fix use of OBJC_RUNTIME_LIB before it was initialised. Thanks to J. Brakensiek for spotting this. --- configure | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure b/configure index bd23b27e5..36a45d1b0 100755 --- a/configure +++ b/configure @@ -2727,6 +2727,7 @@ fi if test -z "$LIBRARY_COMBO"; then LIBRARY_COMBO=`gnustep-config --variable=LIBRARY_COMBO 2>&5` fi +OBJC_RUNTIME_LIB=`echo $LIBRARY_COMBO | tr '-' ' ' | awk '{print $1}'` if test "$OBJC_RUNTIME_LIB" = "ng" -o "$OBJC_RUNTIME_LIB" = "apple"; then nonfragile=yes @@ -6003,7 +6004,6 @@ esac #-------------------------------------------------------------------- # Set Apple/Darwin/OSX/NeXT information for other tests #-------------------------------------------------------------------- -OBJC_RUNTIME_LIB=`echo $LIBRARY_COMBO | tr '-' ' ' | awk '{print $1}'` { $as_echo "$as_me:${as_lineno-$LINENO}: checking the Objective-C runtime" >&5 $as_echo_n "checking the Objective-C runtime... " >&6; } if test "$OBJC_RUNTIME_LIB" = "nx" -o "$OBJC_RUNTIME_LIB" = "apple"; then diff --git a/configure.ac b/configure.ac index d597f5cbd..6fb0d1d6e 100644 --- a/configure.ac +++ b/configure.ac @@ -89,6 +89,7 @@ fi if test -z "$LIBRARY_COMBO"; then LIBRARY_COMBO=`gnustep-config --variable=LIBRARY_COMBO 2>&5` fi +OBJC_RUNTIME_LIB=`echo $LIBRARY_COMBO | tr '-' ' ' | awk '{print $1}'` if test "$OBJC_RUNTIME_LIB" = "ng" -o "$OBJC_RUNTIME_LIB" = "apple"; then nonfragile=yes @@ -1254,7 +1255,6 @@ esac #-------------------------------------------------------------------- # Set Apple/Darwin/OSX/NeXT information for other tests #-------------------------------------------------------------------- -OBJC_RUNTIME_LIB=`echo $LIBRARY_COMBO | tr '-' ' ' | awk '{print $1}'` AC_MSG_CHECKING(the Objective-C runtime) if test "$OBJC_RUNTIME_LIB" = "nx" -o "$OBJC_RUNTIME_LIB" = "apple"; then AC_MSG_RESULT(NeXT) From 3d1e84f6fe5e61453c1bd415a3d878f11891e321 Mon Sep 17 00:00:00 2001 From: Richard Frith-Macdonald Date: Thu, 16 Apr 2020 19:39:32 +0100 Subject: [PATCH 3/6] rewrite code for separating individual cookies from a comma separated header so it better matches what OSX does (and is hopefully a bit simpler)/ --- Source/NSHTTPCookie.m | 238 ++++++++++++++++++++++++++++-------------- 1 file changed, 158 insertions(+), 80 deletions(-) diff --git a/Source/NSHTTPCookie.m b/Source/NSHTTPCookie.m index c719d3fb0..397cc5aa3 100644 --- a/Source/NSHTTPCookie.m +++ b/Source/NSHTTPCookie.m @@ -34,7 +34,7 @@ also contain commas, most notably in the Expires field (which is not quoted and can contain spaces as well). The last key/value does not have to have a semi-colon, so this can be tricky to parse if another cookie occurs - after this (See GSRangeOfCookie). + after this (See GSCookieStrings). */ #import "common.h" @@ -114,7 +114,7 @@ static const unsigned char whitespace[32] = { #define GS_IS_WHITESPACE(X) IS_BIT_SET(whitespace[(X)/8], (X) % 8) static id GSPropertyListFromCookieFormat(NSString *string, int version); -static NSRange GSRangeOfCookie(NSString *string); +static NSMutableArray *GSCookieStrings(NSString *string); @implementation NSHTTPCookie @@ -141,36 +141,42 @@ static NSRange GSRangeOfCookie(NSString *string); forHeader: (NSString *)header andURL: (NSURL *)url { - int version; - NSString *defaultPath, *defaultDomain; - NSMutableArray *a; + int version; + NSString *defaultPath; + NSString *defaultDomain; + NSMutableArray *cookies; + NSUInteger count; if ([header isEqual: @"Set-Cookie"]) - version = 0; + { + version = 0; + } else if ([header isEqual: @"Set-Cookie2"]) - version = 1; + { + version = 1; + } else - return nil; - - a = [NSMutableArray array]; + { + return nil; + } defaultDomain = [url host]; defaultPath = [url path]; if ([[url absoluteString] hasSuffix: @"/"] == NO) - defaultPath = [defaultPath stringByDeletingLastPathComponent]; + { + defaultPath = [defaultPath stringByDeletingLastPathComponent]; + } + cookies = GSCookieStrings(field); + count = [cookies count]; /* We could use an NSScanner here, but this string could contain all sorts of odd stuff. It's not quite a property list either - it has dates and also could have tokens without values. */ - while (1) + while (count-- > 0) { - NSHTTPCookie *cookie; - NSMutableDictionary *dict; - NSString *onecookie; - NSRange range = GSRangeOfCookie(field); + NSHTTPCookie *cookie; + NSMutableDictionary *dict; + NSString *onecookie = [cookies objectAtIndex: count]; - if (range.location == NSNotFound) - break; - onecookie = [field substringWithRange: range]; NS_DURING dict = GSPropertyListFromCookieFormat(onecookie, version); NS_HANDLER @@ -179,18 +185,25 @@ static NSRange GSRangeOfCookie(NSString *string); if ([dict count]) { if ([dict objectForKey: NSHTTPCookiePath] == nil) - [dict setObject: defaultPath forKey: NSHTTPCookiePath]; + { + [dict setObject: defaultPath forKey: NSHTTPCookiePath]; + } if ([dict objectForKey: NSHTTPCookieDomain] == nil) - [dict setObject: defaultDomain forKey: NSHTTPCookieDomain]; + { + [dict setObject: defaultDomain forKey: NSHTTPCookieDomain]; + } cookie = [NSHTTPCookie cookieWithProperties: dict]; if (cookie) - [a addObject: cookie]; + { + [cookies replaceObjectAtIndex: count withObject: cookie]; + } + else + { + [cookies removeObjectAtIndex: count]; + } } - if ([field length] <= NSMaxRange(range)) - break; - field = [field substringFromIndex: NSMaxRange(range)+1]; } - return a; + return cookies; } + (NSArray *) cookiesWithResponseHeaderFields: (NSDictionary *)headerFields @@ -833,72 +846,137 @@ GSPropertyListFromCookieFormat(NSString *string, int version) return AUTORELEASE(dict); } -/* Look for the comma that separates cookies. Commas can also occur in - date strings, like "expires", but perhaps it can occur other places. - For instance, the key/value pair key=value1,value2 is not really - valid, but should we handle it anyway? Definitely we should handle the - perfectly normal case of: - - Set-Cookie: domain=test.com; expires=Thu, 12-Sep-2109 14:58:04 GMT; - session=foo - Set-Cookie: bar=baz - - which gets concatenated into something like: - - Set-Cookie: domain=test.com; expires=Thu, 12-Sep-2109 14:58:04 GMT; - session=foo,bar=baz - -*/ -static NSRange -GSRangeOfCookie(NSString *string) +/* Split a string containing comma seprated cookeie settings into an array + * of individual cookie specifications. + * Look for the comma that separates cookies. Commas can also occur in + * date strings, like "expires", but perhaps it can occur other places. + * For instance, the key/value pair key=value1,value2 is not really + * valid, but should we handle it anyway? Definitely we should handle the + * perfectly normal case of: + * + * Set-Cookie: domain=test.com; expires=Thu, 12-Sep-2109 14:58:04 GMT; + * session=foo + * Set-Cookie: bar=baz + * + * which gets concatenated into something like: + * + * Set-Cookie: domain=test.com; expires=Thu, 12-Sep-2109 14:58:04 GMT; + * session=foo,bar=baz + */ +static NSMutableArray* +GSCookieStrings(NSString *string) { - pldata _pld; - pldata *pld = &_pld; + unsigned char *ptr; + unsigned pos; + unsigned end; NSData *d; - NSRange range; + NSMutableArray *cookies; + unsigned start = 0; + unsigned saved = 0; - /* - * An empty string is a nil property list. - */ - range = NSMakeRange(NSNotFound, NSNotFound); if ([string length] == 0) { - return range; + return nil; } d = [string dataUsingEncoding: NSUTF8StringEncoding]; NSCAssert(d, @"Couldn't get utf8 data from string."); - _pld.ptr = (unsigned char*)[d bytes]; - _pld.pos = 0; - _pld.end = [d length]; - _pld.err = nil; - _pld.lin = 0; - _pld.opt = 0; - _pld.key = NO; - _pld.old = YES; // OpenStep style + ptr = (unsigned char*)[d bytes]; + pos = 0; + end = [d length]; - while (skipSpace(pld) == YES) + cookies = [NSMutableArray arrayWithCapacity: 4]; + while (pos < end) { - if (pld->ptr[pld->pos] == ',') + while (pos < end && isspace(ptr[pos])) { - /* Look ahead for something that will tell us if this is a - separate cookie or not */ - unsigned saved_pos = pld->pos; - while (pld->ptr[pld->pos] != '=' && pld->ptr[pld->pos] != ';' - && pld->ptr[pld->pos] != ',' && pld->pos < pld->end ) - pld->pos++; - if (pld->ptr[pld->pos] == '=') - { - /* Separate comment */ - range = NSMakeRange(0, saved_pos-1); - break; - } - pld->pos = saved_pos; + pos++; } - pld->pos++; - } - if (range.location == NSNotFound) - range = NSMakeRange(0, [string length]); + start = pos; - return range; + while (pos < end) + { + if (ptr[pos] == ',') + { + /* Look ahead for something that will tell us if this is a + * separate cookie or not. We look for something of the form + * ' token =' where a space represents any optional whitespace. + */ + saved = pos; + while (pos < end && isspace(ptr[pos])) + { + pos++; + } + if (pos < end) + { + const char *bad = "()<>@,;:\\\"/[]?={}"; + int c = ptr[pos]; + + /* skip past token characters. + */ + while (c > 32 && c < 128 && strchr(bad, c) == 0) + { + pos++; + if (pos < end) + { + c = ptr[pos]; + } + } + while (pos < end && isspace(ptr[pos])) + { + pos++; + } + if (pos < end && '=' == ptr[pos]) + { + pos = saved + 1; + /* We found a comma separator: so make the text before + * the comma a cooke string as long as it is not empty. + */ + while (saved > start + && isspace(ptr[saved - 1])) + { + saved--; + } + if (saved > start) + { + const char *utf8 = (const char*)(ptr + start); + + ptr[saved] = '\0'; + [cookies addObject: + [NSString stringWithUTF8String: utf8]]; + } + start = saved = pos; + } + } + pos = saved; + } + pos++; + } + } + if (pos > start) + { + /* There was data after the last comma; make it into a cookie string + * as long as it's not empty after removing spaces + */ + saved = pos; + while (saved > start + && isspace(ptr[saved - 1])) + { + saved--; + } + if (saved > start) + { + NSString *str; + + /* There may not be room to add a nul terminator, so we use an + * initialiser which doesn't need one. + */ + str = [[NSString alloc] initWithBytes: ptr + start + length: saved - start + encoding: NSUTF8StringEncoding]; + [cookies addObject: str]; + RELEASE(str); + } + } + return cookies; // The individual cookies } From 6ee0cfff004d806e35e195419944fd7a0ceb3752 Mon Sep 17 00:00:00 2001 From: Richard Frith-Macdonald Date: Thu, 16 Apr 2020 20:19:53 +0100 Subject: [PATCH 4/6] Add testcase for multiple clookies in a header. Fix error parsing a literal string (writing nul terminator to read only memory) --- Source/NSHTTPCookie.m | 20 +++++++++++--------- Tests/base/NSHTTPCookie/basic.m | 6 ++++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Source/NSHTTPCookie.m b/Source/NSHTTPCookie.m index 397cc5aa3..d57cf8208 100644 --- a/Source/NSHTTPCookie.m +++ b/Source/NSHTTPCookie.m @@ -902,7 +902,7 @@ GSCookieStrings(NSString *string) * separate cookie or not. We look for something of the form * ' token =' where a space represents any optional whitespace. */ - saved = pos; + saved = pos++; while (pos < end && isspace(ptr[pos])) { pos++; @@ -939,11 +939,13 @@ GSCookieStrings(NSString *string) } if (saved > start) { - const char *utf8 = (const char*)(ptr + start); + NSString *str = [NSString alloc]; - ptr[saved] = '\0'; - [cookies addObject: - [NSString stringWithUTF8String: utf8]]; + str = [str initWithBytes: ptr + start + length: saved - start + encoding: NSUTF8StringEncoding]; + [cookies addObject: str]; + RELEASE(str); } start = saved = pos; } @@ -966,14 +968,14 @@ GSCookieStrings(NSString *string) } if (saved > start) { - NSString *str; + NSString *str = [NSString alloc]; /* There may not be room to add a nul terminator, so we use an * initialiser which doesn't need one. */ - str = [[NSString alloc] initWithBytes: ptr + start - length: saved - start - encoding: NSUTF8StringEncoding]; + str = [str initWithBytes: ptr + start + length: saved - start + encoding: NSUTF8StringEncoding]; [cookies addObject: str]; RELEASE(str); } diff --git a/Tests/base/NSHTTPCookie/basic.m b/Tests/base/NSHTTPCookie/basic.m index 3c4d4c27f..e89ceda13 100644 --- a/Tests/base/NSHTTPCookie/basic.m +++ b/Tests/base/NSHTTPCookie/basic.m @@ -39,14 +39,15 @@ int main() PASS(cookie == nil, "cookie without path returns nil"); dict = [NSDictionary dictionaryWithObject: - @"S=calendar=R7tjDKqNB5L8YTZSvf29Bg;Expires=Wed, 09-Mar-2011 23:00:35 GMT" + @"S=calendar=R7tjDKqNB5L8YTZSvf29Bg;Expires=Wed, 09-Mar-2011 23:00:35 GMT, " + @"S=xxxxxxxx=R7tjDKqNB5L8YTZSvf29Bg;Expires=Thu, 10-Mar-2011 23:00:35 GMT" forKey: @"Set-Cookie"]; url = [NSURL URLWithString: @"http://www.google.com/calendar/feeds/default/"]; cookies= [NSHTTPCookie cookiesWithResponseHeaderFields: dict forURL: url]; TEST_FOR_CLASS(@"NSArray", cookies, "NSHTTPCookie +cookiesWithResponseHeaderFields: returns an NSArray"); - PASS([cookies count ] == 1, "cookies array contains a cookie"); + PASS([cookies count ] == 2, "cookies array contains two cookies"); cookie = [cookies objectAtIndex: 0]; PASS([[cookie name] isEqual: @"S"], "NSHTTPCookie returns proper name"); PASS([[cookie value] isEqual: @"calendar=R7tjDKqNB5L8YTZSvf29Bg"], @@ -56,6 +57,7 @@ int main() PASS(![cookie isSecure], "Cookie is not secure"); PASS(![cookie isHTTPOnly], "Cookie is not http only"); + cookies = [cookies subarrayWithRange: NSMakeRange(0, 1)]; dict = [NSHTTPCookie requestHeaderFieldsWithCookies: cookies]; PASS_EQUAL([dict objectForKey: @"Cookie"], @"S=calendar=R7tjDKqNB5L8YTZSvf29Bg", From 6d714c8ee14fa83d352a17373937706f548f9149 Mon Sep 17 00:00:00 2001 From: Richard Frith-Macdonald Date: Thu, 16 Apr 2020 22:48:25 +0100 Subject: [PATCH 5/6] Fix for Deadlock in NSOperationQueue #49: If an exception occurs when trying to detach thread, catch it and log it so that locking is not broken by the exception breaking out of the lock protected region. --- Source/NSOperation.m | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/Source/NSOperation.m b/Source/NSOperation.m index 83dd58418..4a8eb5661 100644 --- a/Source/NSOperation.m +++ b/Source/NSOperation.m @@ -1100,9 +1100,18 @@ static NSOperationQueue *mainQueue = nil; || (pending > 0 && internal->threadCount < POOL)) { internal->threadCount++; - [NSThread detachNewThreadSelector: @selector(_thread) - toTarget: self - withObject: nil]; + NS_DURING + { + [NSThread detachNewThreadSelector: @selector(_thread) + toTarget: self + withObject: nil]; + } + NS_HANDLER + { + NSLog(@"Failed to create thread for %@: %@", + self, localException); + } + NS_ENDHANDLER } /* Tell the thread pool that there is an operation to start. */ From 05f442de8b805147fb90072888081211927ac55c Mon Sep 17 00:00:00 2001 From: Richard Frith-Macdonald Date: Thu, 16 Apr 2020 23:04:37 +0100 Subject: [PATCH 6/6] Fix for NSURLResponse does not allow for multiple Set-Cookie headers in the same response #85 ... combine multiple header values as a comma separated list. --- Source/NSURLResponse.m | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Source/NSURLResponse.m b/Source/NSURLResponse.m index 757f8076b..623ca68d3 100644 --- a/Source/NSURLResponse.m +++ b/Source/NSURLResponse.m @@ -114,12 +114,29 @@ typedef struct { { GSMimeHeader *h; + /* Remove existing headers matching the ones we are setting. + */ + e = [(NSArray*)headers objectEnumerator]; + while ((h = [e nextObject]) != nil) + { + NSString *n = [h namePreservingCase: YES]; + + [this->headers removeObjectForKey: n]; + } + /* Set new headers, joining values where we have multiple headers + * with the same name. + */ e = [(NSArray*)headers objectEnumerator]; while ((h = [e nextObject]) != nil) { NSString *n = [h namePreservingCase: YES]; + NSString *o = [this->headers objectForKey: n]; NSString *v = [h fullValue]; + if (nil != o) + { + n = [NSString stringWithFormat: @"%@, %@", o, n]; + } [self _setValue: v forHTTPHeaderField: n]; } }