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

Content-Type for blob images #138

Closed
cinnamon-bun opened this issue Feb 1, 2020 · 7 comments · Fixed by #143 or #169
Closed

Content-Type for blob images #138

cinnamon-bun opened this issue Feb 1, 2020 · 7 comments · Fixed by #143 or #169
Assignees
Labels
bug Something isn't working
Milestone

Comments

@cinnamon-bun
Copy link
Collaborator

Opening an image in a new tab doesn't work well - the browser renders it as text.

I guess we need to figure out the file type on the server and set the Content-Type header. At least for some common image and video types.

Prior work
This has already been done for SVG with #128

Images are normally served from /blob/...hash.... There's another endpoint, /image/64/...hash.... If sharp is installed that resizes and crops images, otherwise it serves them raw. It always sets Content-Type to png no matter the original image type, but browsers seem to do ok even if the data is actually JPG.

Security implications

???

@christianbundy
Copy link
Member

Opening an image in a new tab doesn't work well - the browser renders it as text.

Are you noticing this problem on /blob or /image URLs? Are there specific images that aren't working? I don't think that I've noticed this problem [yet].

It always sets Content-Type to png no matter the original image type, but browsers seem to do ok even if the data is actually JPG.

I think this made sense when Sharp was resizing them, but since Sharp is optional we should double-check that this is only happening if Sharp has turned them into a PNG.

@christianbundy
Copy link
Member

Found the underlying bug I think you're describing. The broken images you were seeing was actually your browser being shown text and being told it was a PNG. 🤦‍♂️ Here's what the error looks like if your browser shows it as text:

TypeError: fakeImage(...).then is not a function
    at /home/christianbundy/src/fraction/oasis/src/index.js:285:32
    at new Promise (<anonymous>)
    at image (/home/christianbundy/src/fraction/oasis/src/index.js:282:14)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:94:5)
    at async /home/christianbundy/src/fraction/oasis/src/index.js:316:16
    at async /home/christianbundy/src/fraction/oasis/src/http.js:28:5
    at async /home/christianbundy/src/fraction/oasis/node_modules/koa-mount/index.js:52:26 {
  expose: true
}

@christianbundy christianbundy added the bug Something isn't working label Feb 2, 2020
@christianbundy christianbundy self-assigned this Feb 2, 2020
@cinnamon-bun
Copy link
Collaborator Author

My bug description was kind of meandering and led to another different bug :D

I meant - find an image someone has posted (not an avatar), right-click the image and open in a new tab.

Example: this post

http://localhost:3000/thread/%25ZLTVRPQoMZ67PHDaH5jvGvs0lhPL3bnlJmtDTI5HXLo%3D.sha256#%25ZLTVRPQoMZ67PHDaH5jvGvs0lhPL3bnlJmtDTI5HXLo%3D.sha256

This image (/blob/ url)

http://localhost:3000/blob/%26%2F3M%2B9WPSPgyBwEVQqhz1UQLafThYi1F3j60PF5usJlI%3D.sha256

It works fine in an <img> tag, but in a tab by itself it tries to render the binary as ascii nonsense.

@christianbundy
Copy link
Member

@cinnamon-bun Thanks for all the merges! ❤️ Does #143 resolve this? For me, that /blob/ URL shows your image, not as ASCII nonsense. Here's what I'm seeing with Curl if that's helpful.

curl -v 'http://localhost:4515/blob/%26%2F3M%2B9WPSPgyBwEVQqhz1UQLafThYi1F3j60PF5usJlI%3D.sha256'
*   Trying ::1:4515...
* TCP_NODELAY set
* connect to ::1 port 4515 failed: Connection refused
*   Trying 127.0.0.1:4515...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 4515 (#0)
> GET /blob/%26%2F3M%2B9WPSPgyBwEVQqhz1UQLafThYi1F3j60PF5usJlI%3D.sha256 HTTP/1.1
> Host: localhost:4515
> User-Agent: curl/7.68.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Length: 197834
< Cache-Control: public,max-age=31536000,immutable
< Content-Disposition: inline; filename="3M+9WPSPgyBwEVQqhz1UQLafThYi1F3j60PF5usJlI=.sha256"
< Content-Security-Policy: default-src 'none'; img-src 'self'; form-action 'self'; media-src 'self'; style-src 'self' 'unsafe-inline'
< X-Frame-Options: SAMEORIGIN
< X-Content-Type-Options: nosniff
< Referrer-Policy: same-origin
< Feature-Policy: speaker 'self'
< Date: Tue, 04 Feb 2020 05:41:02 GMT
< Connection: keep-alive
< 
Warning: Binary output can mess up your terminal. Use "--output -" to tell 
Warning: curl to output it to your terminal anyway, or consider "--output 
Warning: <FILE>" to save to a file.
* Failed writing body (0 != 16384)
* Closing connection 0

@cinnamon-bun
Copy link
Collaborator Author

#143 didn't fix it. I'm seeing those same headers, yep. There's no content-type.

Turns out the browsers act differently:

  • Safari downloads it as a file instead of displaying it
  • Chrome shows it as ascii
  • Firefox shows it as an image

@cinnamon-bun cinnamon-bun reopened this Feb 4, 2020
@cinnamon-bun
Copy link
Collaborator Author

cinnamon-bun commented Feb 4, 2020

The reason I wanted to do this in the first place was to view an image at full size. It'd be nice if images were wrapped in an <a> pointing to just the image URL, for this reason

@christianbundy
Copy link
Member

Hmm. If we don't have another option, I don't mind removing the nosniff protection. 🤷‍♂️ Gotta do what we gotta do.

christianbundy added a commit to christianbundy/oasis that referenced this issue Feb 6, 2020
Problem: We use nosniff to keep the web browser from getting confused
about what kinds of content we're serving in Oasis, but this causes
problems for blob URLs that have arbitrary data.

Solution: Remove nosniff on blob URLs to let the browser figure out what
kind of content we're serving.

Resolves fraction#138
@cinnamon-bun cinnamon-bun added this to the Barebones SSB client milestone Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
2 participants