Bug 16325 - REGRESSION: www.xerox.ru doesn't work
Summary: REGRESSION: www.xerox.ru doesn't work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Alexey Proskuryakov
URL: http://www.xerox.ru/Themes/basic/prod...
Keywords: HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2007-12-06 05:45 PST by Alexey Proskuryakov
Modified: 2007-12-07 02:19 PST (History)
1 user (show)

See Also:


Attachments
proposed fix (4.13 KB, patch)
2007-12-06 05:53 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
revised fix (4.39 KB, patch)
2007-12-06 23:32 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2007-12-06 05:45:22 PST
Steps to reproduce:
1. Open http://www.xerox.ru/Themes/basic/products-index.asp?Folder=739
2. Refresh.

Results: an error page is displayed.

This is caused by document.cookie="" on the page. An empty cookie is stored by NSHTTPCookieStorage and then transmitted as "Cookie: =".

This is a regression in Leopard. I'm not sure if it should be considered an NSHTTPCookieStorage bug, but I have a workaround anyway.
Comment 1 Mark Rowe (bdash) 2007-12-06 05:48:39 PST
I think you should definitely file a Radar for the underlying issue.
Comment 2 Alexey Proskuryakov 2007-12-06 05:53:37 PST
Created attachment 17747 [details]
proposed fix

I'm not particularly happy about having to check for empty cookies all the time - but I don't see a better way.
Comment 3 Alexey Proskuryakov 2007-12-06 06:18:30 PST
Filed <rdar://problem/5632883> for the NSHTTPCookieStorage issue (yes, I will add a comment to code when landing :) ).
Comment 4 David Kilzer (:ddkilzer) 2007-12-06 08:30:40 PST
<rdar://problem/5632997>
Comment 5 Darin Adler 2007-12-06 15:34:54 PST
Comment on attachment 17747 [details]
proposed fix

r=me
Comment 6 Alexey Proskuryakov 2007-12-06 23:32:34 PST
Created attachment 17767 [details]
revised fix

Oops, noticed a couple of problems when landing:
- Fixed Tiger build (no NSUInteger there).
- I didn't really use cookiesForURLFilteredCopy. I could get away with that because the invalid cookie was a per-session one, and didn't need to be filtered out. I'm not sure if we have a real problem with persistent cookies, but I preserved filtering to be safe.

I think I need a new review.
Comment 7 Darin Adler 2007-12-07 00:43:45 PST
Comment on attachment 17767 [details]
revised fix

Our usual approach with NSUInteger is to define it somewhere inside BUILDING_ON_TIGER rather than just using unsigned. See mac/Misc/WebTypesInternal.h for that in WebKit.

Normally we try to avoid autorelease unless we are forced to use it. So we'd typically use a RetainPtr or plain old retain/release on the NSMutableArray.

r=me
Comment 8 Alexey Proskuryakov 2007-12-07 02:19:28 PST
Committed revision 28515.

(In reply to comment #7)
> (From update of attachment 17767 [details] [edit])
> Our usual approach with NSUInteger is to define it somewhere inside
> BUILDING_ON_TIGER rather than just using unsigned. See
> mac/Misc/WebTypesInternal.h for that in WebKit.

  Done (just defined it in CookieJar.mm). I also noticed WebNSUInteger, which is WebKit-only, too.

> Normally we try to avoid autorelease unless we are forced to use it. So we'd
> typically use a RetainPtr or plain old retain/release on the NSMutableArray.

  Done.