Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.9.0-rc2 does not allow text only submissions #3758

Closed
b-meson opened this issue Aug 30, 2018 · 8 comments · Fixed by #3771
Closed

0.9.0-rc2 does not allow text only submissions #3758

b-meson opened this issue Aug 30, 2018 · 8 comments · Fixed by #3771

Comments

@b-meson
Copy link
Contributor

b-meson commented Aug 30, 2018

Description

I believe I found a bug in upgrading from 0.8 to 0.9rc2 in that sources can only submit documents or documents and messages. If I try to send only text or a blank form field I get a "Bad Request, the browser or proxy sent a request that this server could not understand" error.

Steps to Reproduce

First I installed 0.8.0 on hardware and create a journalist. I then ran ./qa-loader.py -m 25. I logged in and and see submissions. I then added the apt-test key and updated sources.list to apt.freedom.press to apt-test.freedom.press. Finally I ran sudo cron-apt -i -s to update to 0.9rc2

Expected Behavior

A source can send text to journalists.

Actual Behavior

Error in the webapp.

Comments

I also enabled apache debug logging and attempted to patch the source_app/ code to log anything related to CSRF violations and I was was not able to trigger a debug log.

@b-meson b-meson mentioned this issue Aug 30, 2018
22 tasks
@redshiftzero
Copy link
Contributor

Thanks for this report, I was unable to reproduce this on rc2, I will debug with the instance having this issue tomorrow with you if that works @b-meson

@eloquence eloquence changed the title 0.9rc2 do not allow text only submissions 0.9.0-rc2 does not allow text only submissions Aug 30, 2018
@kushaldas
Copy link
Contributor

I can confirm the error. I see the same error message.

"Bad Request, the browser or proxy sent a request that this server could not understand"

@kushaldas
Copy link
Contributor

Found the cause will update the ticket + a PR after some coffee.

kushaldas added a commit to kushaldas/securedrop that referenced this issue Aug 31, 2018
Now we check if there is any uploaded file in the request
object before accessing it. This was causing error if the
'fh' key was missing in the request.files.
@emkll
Copy link
Contributor

emkll commented Aug 31, 2018

Based on my testing, this bug does not occur in Dev, nor in Staging environments. It only happens in production (either VMs or hardware). I rebuilt the debs on another computer to rule out the possibility of a bad build, and observed the same error described by @b-meson .

@redshiftzero
Copy link
Contributor

Why does this issue not occur in dev or staging?

@emkll
Copy link
Contributor

emkll commented Aug 31, 2018

request.files and request.form at this point (https://github.com/freedomofpress/securedrop/blob/develop/securedrop/source_app/main.py#L126) in the code returns different behaviors for prod and staging when submitting text:

# staging request.files
ImmutableMultiDict([('fh', <FileStorage>: u' ' ('application/octet-stream')>)])
# staging request.form
ImmutableMultiDict([('msg', u'hello'), ('csrf_token', u'<snip>')])
# prod request.files
ImmutableMultiDict([])
# prod request.form
ImmutableMultiDict([('fh', u''), ('msg', u'hello'), ('csrf_token', u'<snip>')])

emkll added a commit that referenced this issue Aug 31, 2018
Fixes #3758 checks file value before accessing it
@emkll
Copy link
Contributor

emkll commented Aug 31, 2018

Reopening until root cause of variance between staging and prod is addressed

@emkll emkll reopened this Aug 31, 2018
emkll pushed a commit that referenced this issue Aug 31, 2018
Now we check if there is any uploaded file in the request
object before accessing it. This was causing error if the
'fh' key was missing in the request.files.

(cherry picked from commit 4132731)
@conorsch
Copy link
Contributor

Confirmed that staging VMs do not show this issue: I'm able to submit both text and files without issue against staging VMs, configured from release/0.9 (specifically, on 4b86039).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants