Closed
Bug 1065478
Opened 9 years ago
Closed 9 years ago
Regression: POSTs were coming from offline application cache instead of network
Categories
(Core :: Networking: Cache, defect)
Core
Networking: Cache
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: mayhemer, Assigned: mayhemer)
References
Details
Attachments
(1 file)
1.62 KB,
patch
|
jduell.mcbugs
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
We broke the condition for loading from appcache, that has to satisfy responses only for GET or similar methods. Hence POST must go to the network. How this has happen: - when the old cache is used, we assemble the cache v1 cache key that also consist of a POST id - it is then attempted to convert to URL which fails and the complete appcache choice fails => we go to the network The new cache however tries to find the entry only by the URL, so it passes, cache entry is found and loaded from appcache. This is easy to fix, we may simply add a condition to bypass appcache lookup/load when the method is not "GET" or "HEAD".
![]() |
Assignee | |
Comment 1•9 years ago
|
||
It's OK to test for mPostID only since it's non-null for POST and we never run this code for anything else then GET or HEAD otherwise.
Attachment #8487320 -
Flags: review?(jduell.mcbugs)
Updated•9 years ago
|
Attachment #8487320 -
Flags: review?(jduell.mcbugs) → review+
Comment 2•9 years ago
|
||
Honza, so the regression here started in Firefox 31 or 32? I would have thought 32, but we had email from someone saying 31 was broken as well. Also, am I correct in thinking that reverting to cache1 will not actually fix things (since this is nsHttpChannel code not the cache internals that we can switch between)? sicking: Do you know who we can contact to make sure this doesn't break any B2G apps or websites? I'm not sure how common it is to POST from an appcache app to a URI that happens to be in the appcache (but must nevertheless hit the network, since it's a POST: that's my understanding of the bustage scenario here).
Flags: needinfo?(honzab.moz)
Summary: POSTs are coming from offline application cache → Regression: POSTs were coming from offline application cache instead of network
Updated•9 years ago
|
Flags: needinfo?(jonas)
Comment 3•9 years ago
|
||
Sites that serve their login UI with /LOGIN (from the appcache) but then expect a POST to /LOGIN to go to the server are a primary use case here.
![]() |
Assignee | |
Comment 4•9 years ago
|
||
(In reply to Jason Duell (:jduell) from comment #2) > Honza, so the regression here started in Firefox 31 or 32? Only 32, tested. > I would have > thought 32, but we had email from someone saying 31 was broken as well. Bad info. > Also, am I correct in thinking that reverting to cache1 will not actually > fix things (since this is nsHttpChannel code not the cache internals that we > can switch between)? It would fix it since the anti-error is actually in the new cache code. Switching the pref would fix it. > > sicking: Do you know who we can contact to make sure this doesn't break any > B2G apps or websites? I'm not sure how common it is to POST from an > appcache app to a URI that happens to be in the appcache (but must > nevertheless hit the network, since it's a POST: that's my understanding of > the bustage scenario here). Just a note: the thing is that the POST request is performed in a context that is appcached (the channel getd an appcache to use for caching from the associated document).
Flags: needinfo?(honzab.moz)
The Firefox 31 bug was cache related but different: cache-headers for AppCache manifest file needed no-cache, and max-age=0 stopped working suddenly with F31. Apparently other browsers and pre-31 are more lax in this requirement.
I think you could check with Harald Kirschner or Frederic Wenzel.
Flags: needinfo?(jonas)
![]() |
Assignee | |
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/798ea1c2f70c
https://hg.mozilla.org/mozilla-central/rev/798ea1c2f70c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
![]() |
Assignee | |
Comment 9•9 years ago
|
||
[Tracking Requested - why for this release]:
status-firefox32:
--- → affected
status-firefox33:
--- → affected
status-firefox34:
--- → affected
status-firefox35:
--- → fixed
![]() |
Assignee | |
Comment 10•9 years ago
|
||
Comment on attachment 8487320 [details] [diff] [review] v1 Approval Request Comment [Feature/regressing bug #]: cache2enable [User impact if declined]: form POSTs on appcached web pages won't be sent to the server (inability to login, purchase goods, etc...) ; this is against the appcache spec [Describe test coverage new/current, TBPL]: existing appcache functionility is tested with a series of offline application cache tests ; this regression has zero testing coverage since we don't want to add new tests for a dead technology (appcache) [Risks and why]: zero, this is isolated to appcache usage only, any damage to common web content load would be immediately discovered [String/UUID change made/needed]: none
Attachment #8487320 -
Flags: approval-mozilla-beta?
Attachment #8487320 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Updated•9 years ago
|
Attachment #8487320 -
Flags: approval-mozilla-beta?
Attachment #8487320 -
Flags: approval-mozilla-beta+
Attachment #8487320 -
Flags: approval-mozilla-aurora?
Attachment #8487320 -
Flags: approval-mozilla-aurora+
Comment 12•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/c12a3c6193b3 https://hg.mozilla.org/releases/mozilla-beta/rev/6c39ccb686a5
Comment 13•9 years ago
|
||
thanks for fixing - since HTML5 AppCache is referred to as a dead technology above, what do you recommend we look into as a replacement?
![]() |
Assignee | |
Comment 14•9 years ago
|
||
(In reply to Kimon from comment #13) > thanks for fixing - since HTML5 AppCache is referred to as a dead technology > above, what do you recommend we look into as a replacement? Service Workers. It's being implemented in Gecko now.
You need to log in
before you can comment on or make changes to this bug.
Description
•