Skip to content
This repository has been archived by the owner on Feb 8, 2022. It is now read-only.

set Accept header to */* if raw param is set #15

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dtao
Copy link

@dtao dtao commented Dec 21, 2013

The (undocumented) raw parameter says to me, "I want to get arbitrary non-JSON data from this URL; please wrap it for me." However if we explicitly set the Accept header to 'application/json' we might get a 406 back from the external service. If the user explicitly set the raw param, it makes sense to accept everything, right?

The (undocumented) raw parameter says to me, "I want to get arbitrary non-JSON data from this URL; please wrap it for me." However if we explicitly set the Accept header to 'application/json' we might get a 406 back from the external service. If the user explicitly set the raw param, it makes sense to accept everything, right?
@afeld
Copy link
Owner

afeld commented Dec 21, 2013

Good call! Mind adding a test?

dtao added a commit to dtao/jsonp that referenced this pull request Dec 23, 2013
@afeld
Copy link
Owner

afeld commented Jan 22, 2014

Now thinking that it should pass the Accept header from the incoming request (so basically normal proxying)... sound reasonable?

@dtao
Copy link
Author

dtao commented Feb 6, 2014

Sure, I guess that makes sense. In that case maybe you could drop the raw param altogether (and infer whether the user wants JSON back by looking at the Accept header)?

@afeld
Copy link
Owner

afeld commented Feb 6, 2014

My main concern there would be a) that I almost always use my browser to test API calls, which request text/html. Most APIs will only respond with JSON anyway so maybe it's not a huge deal, but I don't want to confuse anyone.

Also, I'd want to be confident that doesn't (further) open up any XSS vulnerabilities.

@dtao
Copy link
Author

dtao commented Feb 6, 2014

OK, fine by me then. Not that you needed my approval ;)

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

Successfully merging this pull request may close these issues.

2 participants