Page MenuHomePhabricator

Remove the developer-specific CSRF help in phabricator_form()
ClosedPublic

Authored by epriestley on Apr 15 2014, 4:11 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, May 3, 4:08 AM
Unknown Object (File)
Mon, Apr 29, 4:28 PM
Unknown Object (File)
Fri, Apr 26, 1:28 AM
Unknown Object (File)
Thu, Apr 25, 9:19 PM
Unknown Object (File)
Wed, Apr 24, 10:32 PM
Unknown Object (File)
Tue, Apr 16, 9:46 AM
Unknown Object (File)
Mar 15 2024, 7:53 AM
Unknown Object (File)
Feb 18 2024, 6:44 AM
Subscribers

Details

Summary

Fixes T4802. For context, see T1921.

Originally (in T1921), a developer ran into an issue where rendering phabricator_form() with an absolute URI confusingly dropped CSRF tokens, and it wasn't obvious why. This is a security measure, but at the time it wasn't very clear how all the pieces fit together. To make it more clear, we:

  1. expanded the exception text in developer mode to include a description of this issue; and
  2. added an exception in developer mode when rendering a form like this.

However, (2) causes some undesirable interactions for file downloads. In particular, if:

  • developer mode is on; and
  • there's no alternate file domain configured; and
  • you try to download a file...

...we produce CDN URIs that are fully-qualified, and you get the exception from (2) above.

This is kind of a mess, and producing fully-qualified CDN URIs in all cases is simple and clear and desirable. To resolve this, just revert (2). We still have the clarification from (1) above and this hasn't caused further issues, so I think that's sufficient. This is a rare issue anyway and not particularly serious or error prone (at worst, a bit confusing and annoying, but hopefully easy to understand and resolve after the changes in (1)).

Test Plan

With develper mode and no alternate file domain, downloaded files from Files.

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley retitled this revision from to Remove the developer-specific CSRF help in phabricator_form().
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
This revision is now accepted and ready to land.Apr 15 2014, 5:05 PM
epriestley updated this revision to Diff 20829.

Closed by commit rP04c07a7a7b6d (authored by @epriestley).