Open Bug 479296 Opened 14 years ago Updated 5 months ago

Saving POST result page silently resends POST data if page no longer in cache

Categories

(Firefox :: File Handling, defect, P3)

defect

Tracking

()

People

(Reporter: bzbarsky, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: uiwanted)

Attachments

(1 file)

STEPS TO REPRODUCE:

1)  Load a POST result page
2)  Clear the cache.
3)  File > Save As
4)  Save as "HTML only"
5)  Either make an HTTP log as you do this, or have access to the server, or use
    a server that produces different output on every POST.

EXPECTED RESULTS: No silent repost

ACTUAL RESULTS: silent repost

Now that bug 84106 is fixed, we'll at least _try_ to hit the cache before reposting, but if the content has expired from cache we'll happily redo the POST.  If you're saving a purchase confirmation page, you might end up repeating the purchase as a result.

For history traversal, we make sure to never do this.  If the POST page is not cached, we put up a dialog asking the user whether to resend the POST data.

Is that the behavio we want here?  Do we want something else (like just aborting the save operation)?
What happens if you Ctrl A, right-click -> View Selection Source, then in the "DOM Source of Selection" window do File -> Save Page As...?
If that doesn't repost could this be the wanted behaviour?
> What happens if

That calls ViewSourceSavePage(), which does a GET on the original page URL.  So it's completely broken for POST pages no matter what.  Want to file a bug on that?
(In reply to comment #2)
I think that's bug 469302, is it fixed on trunk and branch?
No, it is NOT bug 469302.  That bug is about the source shown.  Comment 2 is about the source saved (which is distinctly different from what's shown).  Neither has anything to do with this bug.
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
"Please pay me $5 via PayPal and save two copies of the confirmation page -- one for your files and one to send to me."
Blocks: 485196
Need UI spec; see end of comment 0.
Keywords: uiwanted
Some development references:

* The place where the "web browser persist" component sets the channel
   properties, to control whether an URL is loaded from the cache:

http://mxr.mozilla.org/mozilla-central/source/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp#1194

* The place where the docshell sets the HTTP channel properties, to control
   whether an URL is loaded *only* from the cache:

http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#7532

* The docshell code that handles the case where a document was not found in
   the cache, and asks the user whether to repost:

http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsWebShell.cpp#1198

* The actual repost dialog implementation:

http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#8991


The open question from comment 0 is still if, when "Save As" requires a repost,
we should:
 * Put up a dialog asking the user whether to resend the POST data
 * Abort the save
 * Something else
Noting related bug 177329.
Depends on: 486921
This should block bug 288462

@Boris, is this issue still reproducible or it can be closed since there are no new updates for a long time?
Thanks!

Flags: needinfo?(bzbarsky)

There are steps in comment 0 that could be used to check if this issue still applies or not.

I tested this issue using the steps from comment 0. Can you please confirm that silent repost refers to the post that appears after Saving page as HTML? I attached a screenshot with my results after saving page as HTML and no other actions on the page.

Attached image 2021-07-19_16h14_17.png
Flags: needinfo?(continuation)

It's hard to say what's going on there, especially given the serviceworker. But also, as far as I know the fetches "save as" does don't show up in the devtools network panel anyway.

I just tried those steps with https://www.randomresult.com/num.php and I definitely see a new POST happening when I save, in that the saved data does not match the displayed data, if I clear cache before saving.

I believe I also see a new POST happening on "view source" (again, based on examination of the actual data in the source and whether it matches the original page), which is really broken. I tested with "view_source.tab" both true and false and see the same behavior in both cases.

Flags: needinfo?(bzbarsky) → needinfo?(gijskruitbosch+bugs)
QA Whiteboard: [qa-not-actionable]

(In reply to Boris Zbarsky [:bzbarsky] from comment #14)

It's hard to say what's going on there, especially given the serviceworker. But also, as far as I know the fetches "save as" does don't show up in the devtools network panel anyway.

They do show up in the browser toolbox's network panel. :-)

I just tried those steps with https://www.randomresult.com/num.php and I definitely see a new POST happening when I save, in that the saved data does not match the displayed data, if I clear cache before saving.

Yeah, same. This still seems broken.

I believe I also see a new POST happening on "view source" (again, based on examination of the actual data in the source and whether it matches the original page), which is really broken. I tested with "view_source.tab" both true and false and see the same behavior in both cases.

I filed bug 1721580 for the view source case.

In terms of what to do here, I think a warning along the same lines as the history navigation case would make sense. The docshell code for the history navigation case is here:

https://searchfox.org/mozilla-central/rev/699174544b058f13f02e7586b3c8fdbf438f084b/docshell/base/nsDocShell.cpp#11747-11753

and the relevant prompt is completely encapsulated in a simple idl API call that we can easily reuse

However, it is not clear to me how the webbrowserpersist code (which sets up the channel to make the request for the "HTML only" save) would know whether or not there'd be a cache hit for the request. Nihanth, do you know?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(nhnt11)
Flags: needinfo?(continuation)

Whoops, meant to look at this earlier and then it got lost in backlog when I went on vacation.

I think we have an opportunity to do some logic in nsWebBrowserPersist::SaveURIInternal, which seems to do the actual loads for each URI. There are some load flags being set (defaults). I wonder if we could try to do a cache-only load, and if it fails, prompt before doing a network load?

I didn't dig further than that because I couldn't reproduce - clearing all data and then saving the page didn't show an extra POST request nor did I see newly generated random numbers in the saved page. Any chance something "fixed" this already in the 2 months that I didn't respond here, I wonder? I can ask in the necko meeting tomorrow...

Flags: needinfo?(nhnt11)

clearing all data and then saving the page didn't show an extra POST request

Just to double-check: did you save as "Web Page, HTML only", or "Web Page, complete"? The default behavior in a clean profile is "Web Page, complete", iirc. The codepath that has the bug is "Web Page, HTML only".

Flags: needinfo?(nhnt11)

You're right, I missed that somehow. I can reproduce with https://www.randomresult.com/num.php.

I think my suspicion in comment 16 that this might be actionable still holds, but my queue is pretty full right now so I'll have to let this go through triage.

Flags: needinfo?(nhnt11)
Severity: critical → S2
Priority: -- → P3
Severity: S2 → S3
You need to log in before you can comment on or make changes to this bug.