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

ipfs get: set correct content-type on resp #2004

Closed
wants to merge 1 commit into from
Closed

Conversation

rht
Copy link
Contributor

@rht rht commented Nov 24, 2015

closes #1824

License: MIT
Signed-off-by: rht <rhtbot@gmail.com>
@jbenet jbenet added the status/in-progress In progress label Nov 24, 2015
@rht rht added the RFCR label Nov 24, 2015
@whyrusleeping
Copy link
Member

mmm... i see you PRing against master... i'll allow this one though. fairly trivial.

LGTM

@whyrusleeping whyrusleeping removed the status/in-progress In progress label Nov 28, 2015
@@ -232,7 +234,9 @@ func sendResponse(w http.ResponseWriter, r *http.Request, res cmds.Response, req
if _, ok := res.Output().(io.Reader); ok {
// set streams output type to text to avoid issues with browsers rendering
// html pages on priveleged api ports
mime = "text/plain"
if mime != mimeTypes[cmds.Tar] && mime != mimeTypes[cmds.Gzip] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe can make it something like:

if !xssSafeMimeType(mime) {
  mime = mimeTypes[cmds.Text]
}

and elsewhere something like:

var xssSafeMimeTypes = []string{
  mimeTypes[cmds.Tar],
  mimeTypes[cmds.GZip],
  mimeTypes[cmds.Text],
  mimeTypes[cmds.JSON],
}

func xssSafeMimeType(mime string) {
  for _, t := range xssSafeMimeTypes {
    if t == mime {
      return true
    }
  }
  return false
}

to make it clear why this is happening, and easier to broaden the safety boundary, in a single place.

this probably could be a separate package, and may already exist somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No such package exists elsewhere yet.

@jbenet
Copy link
Member

jbenet commented Dec 1, 2015

one comment above, otherwise LGTM

@ghost ghost added topic/commands Topic commands and removed RFCR labels Dec 22, 2015
@whyrusleeping
Copy link
Member

@rht update here?

@rht
Copy link
Contributor Author

rht commented Jan 30, 2016

@rht doesn't have commit access

@rht rht closed this Jan 30, 2016
@whyrusleeping whyrusleeping deleted the get-resp-header branch January 30, 2016 04:33
RichardLitt added a commit that referenced this pull request May 12, 2016
This is a version of #2004, with CR from @jbenet integrated in, and should close #1824

License: MIT
Signed-off-by: Richard Littauer <richard.littauer@gmail.com>
Kubuxu pushed a commit that referenced this pull request May 31, 2016
This is a version of #2004, with CR from @jbenet integrated in, and should close #1824

License: MIT
Signed-off-by: Richard Littauer <richard.littauer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/commands Topic commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTP API - Content-Type returned by get command should be application/x-tar
3 participants