Bug 16325

Summary:

REGRESSION: www.xerox.ru doesn't work

Product:

WebKit

Reporter:

Alexey Proskuryakov <ap>

Component:

Page Loading

Assignee:

Alexey Proskuryakov <ap>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

mrowe

Priority:

Keywords:

HasReduction, InRadar, Regression

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.4

URL:

http://www.xerox.ru/Themes/basic/products-index.asp?Folder=739

Attachments:

Description	Flags
proposed fix	none
revised fix	darin: review+

Alexey Proskuryakov

Reported 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.

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Mark Rowe (bdash)

Comment 1 2007-12-06 05:48:39 PST

I think you should definitely file a Radar for the underlying issue.

Alexey Proskuryakov

Comment 2 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.

Alexey Proskuryakov

Comment 3 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 :) ).

David Kilzer (:ddkilzer)

Comment 4 2007-12-06 08:30:40 PST

<rdar://problem/5632997>

Darin Adler

Comment 5 2007-12-06 15:34:54 PST

Comment on attachment 17747 [details] proposed fix r=me

Alexey Proskuryakov

Comment 6 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.

Darin Adler

Comment 7 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

Alexey Proskuryakov

Comment 8 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.

Note You need to log in before you can comment on or make changes to this bug.