Teach the parser about the integrity attribute

RESOLVED FIXED in mozilla36

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: francois, Assigned: francois)

Tracking

unspecified
mozilla36
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
This attribute is defined by the sub-resource integrity specification:

https://w3c.github.io/webappsec/specs/subresourceintegrity/
(Assignee)

Updated

4 years ago
Assignee: nobody → francois
Status: NEW → ASSIGNED
(Assignee)

Comment 1

4 years ago
Created attachment 8523647 [details] [diff] [review]
Add integrity attribute

Henri, I'm putting this patch up for review here because I've tested it in bug 992096 and it looks very much in line with similar commits I've seen in the hg history.

However, there are two caveats:

1. I haven't been able to figure out how to run tests in the htmlparser repo.
2. I don't know where Doug got the magic number from.

I'd love to be educated on these two points because I couldn't find any documentation as to what the magic numbers are referring to, and running "mvn test" said that "Tests are skipped".
Attachment #8523647 - Flags: review?(hsivonen)
Comment on attachment 8523647 [details] [diff] [review]
Add integrity attribute

(In reply to François Marier [:francois] from comment #1)
> Created attachment 8523647 [details] [diff] [review]
> Add integrity attribute
> 
> Henri, I'm putting this patch up for review here because I've tested it in
> bug 992096 and it looks very much in line with similar commits I've seen in
> the hg history.
> 
> However, there are two caveats:
> 
> 1. I haven't been able to figure out how to run tests in the htmlparser repo.

To run the tests, you need a checkout of https://github.com/html5lib/html5lib-tests/ and then you give the relevant test file paths as arguments to TreeTester or TokenizerTester.

> 2. I don't know where Doug got the magic number from.

The magic number comes from uncommenting some code in AttributeName.java and running the file as a Java program. Assuming that Doug generated the number that way, it should be OK, so r+.

The number is used for efficient lookup of the objects.
Attachment #8523647 - Flags: review?(hsivonen) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Hi Francois,

the patch failed to apply:

applying bug1100206.patch
unable to find 'src/nu/validator/htmlparser/impl/AttributeName.java' for patching
3 out of 3 hunks FAILED -- saving rejects to file src/nu/validator/htmlparser/impl/AttributeName.java.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir

could you take a look at this ? Thanks!

Also if possible could you include a try link thanks!
Flags: needinfo?(francois)
Keywords: checkin-needed
(Assignee)

Comment 4

4 years ago
Sorry, I forgot to say that this patch is not for mozilla-central, but rather for https://hg.mozilla.org/projects/htmlparser/

I'm guessing I shouldn't have used the checkin-needed keyword right?
Flags: needinfo?(francois) → needinfo?(cbook)
(In reply to François Marier [:francois] from comment #4)
> Sorry, I forgot to say that this patch is not for mozilla-central, but
> rather for https://hg.mozilla.org/projects/htmlparser/
> 
> I'm guessing I shouldn't have used the checkin-needed keyword right?

good question, hsivonen could you do the checkin ?
Flags: needinfo?(cbook) → needinfo?(hsivonen)
https://hg.mozilla.org/mozilla-central/rev/8f6cfc3bbdf2
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
(Assignee)

Comment 7

4 years ago
It looks like an empty patch was committed to mozilla-central by mistake:

  $ hg show 216205:8f6cfc3bbdf2
  changeset:   216205:8f6cfc3bbdf2
  user:        Francois Marier <francois@mozilla.com>
  date:        Sun Nov 16 20:12:00 2014 +0100
  description:
  Mozilla bug 1100206 - Teach the parser about the integrity attribute

I guess there's no need to revert it since it's an empty commit, but this is meant to go on the htmlparser repo.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 8538441 [details] [diff] [review]
What actually landed on htmlparser

Did the previously attached patch really come from code generation? The magic number isn't sorted right. Just to be sure, I ran AttributeName generation again and landed the re-generated result:
https://hg.mozilla.org/projects/htmlparser/rev/903fa49d6fa8

Attaching what actually got landed on htmlparser.
Attachment #8523647 - Attachment is obsolete: true
Flags: needinfo?(hsivonen)
Attachment #8538441 - Flags: review+
Created attachment 8538443 [details] [diff] [review]
Generated code to land on m-c

Here's the m-c patch. I put you in as the hg user, since there's really no authorship from me here. I just re-ran the generator. I didn't land it on m-c, though, in case you want to land it yourself.
Attachment #8538443 - Flags: review+
(Assignee)

Comment 10

4 years ago
Thanks Henri!

I don't actually have push access to mozilla-central, so I'll mark the "Generated code to land on m-c" as "checkin-needed.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7b7cbab2c5e3
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.