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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix permission check for net #2373

Merged
merged 6 commits into from May 17, 2019

Conversation

2 participants
@bartlomieju
Copy link
Contributor

commented May 17, 2019

Closes #2371

It turns out we were checking for "listen", "dial" and "accept" addresses in net permissions 馃槄

I removed check in op_accept because we need to first issue op_listen and there is already check there.

bartlomieju added some commits May 17, 2019

fmt
@ry

This comment has been minimized.

Copy link
Collaborator

commented May 17, 2019

Thanks! Is it possible to add a test to tools/complex_permissions_test.py that demos this fix? (Presumably, for example, listen wasn't properly obeying --allow-net arguments before.)

@bartlomieju

This comment has been minimized.

Copy link
Contributor Author

commented May 17, 2019

Sorry @ry, should have started with tests in the first place, turns out my initial solution was wrong 馃槈 I guess it's worth mentioning in the docs that --allow-net=addr1,addr2 allows to access addr1 and addr2 before they are resolved

Show resolved Hide resolved tools/complex_permissions_test.py Outdated
@ry

ry approved these changes May 17, 2019

Copy link
Collaborator

left a comment

LGTM - thanks for this fix!

@ry ry merged commit 7219787 into denoland:master May 17, 2019

3 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@bartlomieju bartlomieju deleted the bartlomieju:fix-net_perms branch May 17, 2019

piscisaureus added a commit to piscisaureus/deno that referenced this pull request May 23, 2019

bartlomieju added a commit to bartlomieju/deno that referenced this pull request May 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.