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

Use JSON.parse to limit exposure #75

Merged
merged 1 commit into from
Oct 15, 2014

Conversation

chancancode
Copy link
Contributor

As far as I can tell, there are no pressing reasons for this change (as in, this is
not addressing an immediate security threat). It's just a preventive measure to
reduce unnecessary exposure.


Using JSON.load on untrusted input is considered unsafe. While the MIME type
definition files would presumably come from a trusted source, there doesn't seem
to be a need for the "extra" stuff that JSON.load does in here, so switching
over to the safer JSON.parse API should help to reduce exposure.

Using `JSON.load` on untrusted input is considered unsafe. While the MIME type
definition files would presumably come from a trusted source, there doesn't seem
to be a need for the "extra" stuff that `JSON.load` does in here, so switching
over to the safer `JSON.parse` API should help to reduce exposure.
@halostatue halostatue added the Bug label Oct 15, 2014
@halostatue halostatue modified the milestones: 2.4, 2.5 Oct 15, 2014
@halostatue halostatue self-assigned this Oct 15, 2014
@halostatue
Copy link
Member

Thanks. I’ll get this merged soon.

halostatue added a commit that referenced this pull request Oct 15, 2014
Use `JSON.parse` to limit exposure
@halostatue halostatue merged commit ae1d160 into mime-types:master Oct 15, 2014
@halostatue halostatue modified the milestones: 2.4, 2.5 Oct 15, 2014
@halostatue
Copy link
Member

As a side note: this increased performance another 20%, so it’s worthwhile on its own. With this and #74, registry loading is now about 55% faster (rake benchmark:load went from 18s to 10s with #74 and from 10s to 8s with this change).

@jeremy
Copy link

jeremy commented Oct 15, 2014

👌

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

Successfully merging this pull request may close these issues.

3 participants