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

File thumbnails sent with wrong Content-Type #9612

Closed
hypeJunction opened this Issue Apr 4, 2016 · 8 comments

Comments

Projects
None yet
2 participants
@hypeJunction
Contributor

hypeJunction commented Apr 4, 2016

Currently file thumbnails are generated as jpeg, but the thumbnail file extension and Content-Type sent is inherited from the source file.

The correct Content-Type is sent in 2.x (serve-file analyzes the file), but previous versions including 2.1 always send the original file's Content-Type.

We also should change the thumb file extensions to jpg for consistency, and in case the file is served elsewhere.

  • wait for #9608 to be merged
@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Apr 4, 2016

Member

Currently we're just lucky our MIME detectors are reading the file header, because the serve-file handler has no knowledge of the original file.

Member

mrclay commented Apr 4, 2016

Currently we're just lucky our MIME detectors are reading the file header, because the serve-file handler has no knowledge of the original file.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Apr 4, 2016

Member

Is there risk 3rd party thumbs are relying on thumb extensions matching the original extension?

Member

mrclay commented Apr 4, 2016

Is there risk 3rd party thumbs are relying on thumb extensions matching the original extension?

@mrclay mrclay self-assigned this Apr 4, 2016

@mrclay mrclay changed the title from Flaw in file thumbnail naming convention to File thumbnails sent with wrong Content-Type Apr 4, 2016

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Apr 4, 2016

Member

See the new title and desc on this ticket.

Member

mrclay commented Apr 4, 2016

See the new title and desc on this ticket.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Apr 4, 2016

Member

Browsers just assume wrong image types are sent all the time and fix it, but we should fix this in 1.12. I'm working on this now.

Member

mrclay commented Apr 4, 2016

Browsers just assume wrong image types are sent all the time and fix it, but we should fix this in 1.12. I'm working on this now.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Apr 4, 2016

Member

When we re-upload (edit) a file, we keep the old filename (!?). And if the new file isn't an image, we leave the old thumbs in place (!?). If get_resized_image_from_existing_file can't handle the given image, we keep the old thumbnails.

Member

mrclay commented Apr 4, 2016

When we re-upload (edit) a file, we keep the old filename (!?). And if the new file isn't an image, we leave the old thumbs in place (!?). If get_resized_image_from_existing_file can't handle the given image, we keep the old thumbnails.

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Apr 4, 2016

Contributor

I have an open ticket for that somewhere
On Apr 4, 2016 9:30 PM, "Steve Clay" notifications@github.com wrote:

When we re-upload (edit) a file, we keep the old filename (!?). And if the
new file isn't an image, we leave the old thumbs in place (!?).


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#9612 (comment)

Contributor

hypeJunction commented Apr 4, 2016

I have an open ticket for that somewhere
On Apr 4, 2016 9:30 PM, "Steve Clay" notifications@github.com wrote:

When we re-upload (edit) a file, we keep the old filename (!?). And if the
new file isn't an image, we leave the old thumbs in place (!?).


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#9612 (comment)

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Apr 4, 2016

Contributor

Can't find it. The bigger problem is that we allow reuploads with different mime and keep the old filename!

Contributor

hypeJunction commented Apr 4, 2016

Can't find it. The bigger problem is that we allow reuploads with different mime and keep the old filename!

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Apr 4, 2016

fix(file): better uploaded file handling and thumbnail serving
Uploaded files now always have a new name with the client-given extension,
even when replacing a previous upload. Thumbnails files now have the `.jpg`
extension and are served with the `image/jpeg` Content-Type.

Fixes #9612
Fixes #9267
Fixes #6677
@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay
Member

mrclay commented Apr 4, 2016

PR #9615

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Apr 4, 2016

fix(file): better uploaded file handling and thumbnail serving
Uploaded files now always have a new name with the client-given extension,
even when replacing a previous upload. Thumbnails files now have the `.jpg`
extension and are served with the `image/jpeg` Content-Type.

Fixes #9612
Fixes #9267
Fixes #6677

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Apr 4, 2016

fix(file): better uploaded file handling and thumbnail serving
Uploaded files now always have a new name with the client-given extension,
even when replacing a previous upload. Thumbnails files now have the `.jpg`
extension and are served with the `image/jpeg` Content-Type.

If the file is recognized as an image, but thumbnails can't be created, we
no longer allow thumbs from the previous file to be re-used.

Fixes #9612
Fixes #9267
Fixes #6677

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Apr 4, 2016

fix(file): better uploaded file handling and thumbnail serving
Uploaded files now always have a new name with the client-given extension,
even when replacing a previous upload.

Thumbnail JPEG files now have the `.jpg` extension and are served with the
correct Content-Type. If a plugin happened to have created their own non-JPEG
thumbs, they'd now be served with the correct MIME (instead of the original
file's MIME type).

If the file is recognized as an image, but thumbnails can't be created, we
no longer allow thumbs from the previous file to be re-used.

Fixes #9612
Fixes #9267
Fixes #6677

@mrclay mrclay closed this in 72140cf Apr 5, 2016

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