Skip to content
This repository has been archived by the owner. It is now read-only.

small number of avatar uploads rejected for >1MB size #148

Closed
jrgm opened this issue Sep 22, 2015 · 10 comments
Closed

small number of avatar uploads rejected for >1MB size #148

jrgm opened this issue Sep 22, 2015 · 10 comments

Comments

@jrgm
Copy link
Contributor

@jrgm jrgm commented Sep 22, 2015

STR

  1. install netpbm to generate an uncompressible png (white noise)
  2. rawtoppm < /dev/urandom > rnd.ppm; pnmtopng < rnd.ppm > rnd.png
  3. try to upload rnd.png as your avatar

Expected: the server will return 'HTTP/1.1 413 Request Entity Too Large'; reflect that to the user in the UI

Actual: Fx network console doesn't show an HTTP response to the /v1/avatar/upload request; the UI shows 'System unavailable, try again soon'.

@edwindotcom
Copy link

@edwindotcom edwindotcom commented Sep 22, 2015

jrgm to increase limit for investigation.

@zaach
Copy link
Contributor

@zaach zaach commented Sep 22, 2015

jrgm noted that the browser is not actually receiving the 413 error although nginx is sending it. The browser will need to see the 413 in order for us to give better messaging.

@seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Sep 23, 2015

I... what? 413 is being sent, but not the browser isn't receiving it? How can that be?

@jrgm
Copy link
Contributor Author

@jrgm jrgm commented Sep 25, 2015

I have a fix to raise the limit on nginx, but this will also need a change in fxa-profile-server.

The change needed is to add maxBytes: 1200000 here, and here.

However, in writing tests for this, it turns out the current tests test/lib/mock.js is tied to a specific image and size here and here

@jrgm jrgm assigned seanmonstar and unassigned jrgm Sep 25, 2015
@rfk rfk added this to the FxA-0: quality milestone Sep 30, 2015
@seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Oct 19, 2015

This was assigned to me and put in now, but I don't see what the specific action is...

@seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Oct 19, 2015

Since this issue is larger than the specific action needed in 'now', I've created #158, which 'blocks' this. I'll move this into next, and fix #158.

@rfk
Copy link
Member

@rfk rfk commented Oct 20, 2015

Is there any further action required on this bug once #158 lands? If not, I suggest we just close it out in favour of the more action-oriented one.

@seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Oct 20, 2015

@rfk it seems this issue is to find out why avatars are being rejected. so next steps are likely to increase the maxSize in production a little, and then monitor if the errors go away, etc.

@rfk
Copy link
Member

@rfk rfk commented Oct 30, 2015

IIUC the next action here is to confirm whether train-48 deployment helped the situation any

@jrgm
Copy link
Contributor Author

@jrgm jrgm commented Oct 30, 2015

this is fixed

@jrgm jrgm closed this Oct 30, 2015
@jrgm jrgm removed the waffle:next label Oct 30, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants