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

Security checklist #33

Merged
merged 4 commits into from
Sep 12, 2017
Merged

Security checklist #33

merged 4 commits into from
Sep 12, 2017

Conversation

iherman
Copy link
Member

@iherman iherman commented Sep 6, 2017

@iherman iherman requested a review from tripu September 6, 2017 15:51
@tripu tripu self-assigned this Sep 6, 2017
@iherman
Copy link
Member Author

iherman commented Sep 7, 2017

@tripu I have added some checks on URL-s, inspired by a script done by Bratt Smith (and added to one of my Python based CGI scripts): http://dev.w3.org/2004/PythonLib-IH/checkremote.py.

It does not reproduce the whole thing; to be honest I did not follow all the details of what he did, but I believe I did what is really of importance here. See the additional comments in the code (check_url function in io.js). I also added checks to be sure that what is returned for a fetch is of the type that one expects (plain text or JSON, respectively). Again, the changes are in io.js

@tripu tripu added this to WIP in Plan Sep 12, 2017
Copy link
Member

@tripu tripu left a comment

Choose a reason for hiding this comment

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

Your changes look mostly good to me, so let's merge this one. I'll have to come back with help for some of the few questions still open, but I think we're almost there.

@tripu tripu merged commit 81e9ae9 into master Sep 12, 2017
@tripu tripu deleted the security-checklist branch September 12, 2017 15:20
@tripu tripu moved this from WIP to ✓ Done in Plan Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Plan
✓ Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants