script: Implement base-uri CSP check#42272
Conversation
TimvdLippe
left a comment
There was a problem hiding this comment.
Thanks for taking this on! Some nits, but generally LGTM.
P.S. You have picked up a few starter issues now and have proven you are capable of contributing features to Servo. For the next project, do you mind taking on a slightly bigger one? Then we can leave the starter issues for those folks unfamiliar with Servo. Looking forward to hearing your interest and we can help you figure out an appropriate task!
Thanks! I'd definitely be interested in taking on something more challenging. I really, really like Servo. It's so interesting and has taught me a lot, and I hope to contribute more. That said, I should mention that I'm currently still a student, so my availability can be a bit unpredictable. And when completing some past issues, I often actually spent more time than it appeared to understand and learn those new knowledge (if there was a clear direction to let me know what to learn systematically, it would be great). I don't want to commit to something and then end up blocking progress. So if there's a task that's not super time-sensitive, I'd really be happy to give it a shot. |
There was a problem hiding this comment.
The reason for the failure here seems to be that the port_part_match function in the content-security-policy crate performs a direct string comparison, which results in "08000" != "8000"
There was a problem hiding this comment.
I'm not sure if my understanding is correct, but according to the spec the function points to, the port number seems to should be interpreted as a decimal number?
There was a problem hiding this comment.
Yup I think you found a bug in their code! Feel free to send them a PR with a fix (and a test preferably). When we update the CSP crate we can also update this test expectation.
Signed-off-by: WaterWhisperer <[email protected]>
f2c7708 to
fb04116
Compare
TimvdLippe
left a comment
There was a problem hiding this comment.
No worries about time-sensitive work. We have plenty of contributors that work for a long time on projects and that's all fine. We appreciate everybody's input, regardless of how long it takes. In fact, taking time to learn things is a great use of your time. So that's encouraged as well.
I recommend you check out the issues with label E-candidate-for-mentoring (but not those with E-very-complex) and let us know. Then we can help you further along.
There was a problem hiding this comment.
Yup I think you found a bug in their code! Feel free to send them a PR with a fix (and a test preferably). When we update the CSP crate we can also update this test expectation.
Understood, thanks! |
Testing:
./mach test-wpt /content-security-policy/base-uriFixes: #42261