Closed Bug 1100206 Opened 5 years ago Closed 5 years ago

Teach the parser about the integrity attribute

Categories

(Core :: HTML: Parser, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: francois, Assigned: francois)

References

Details

Attachments

(2 files, 1 obsolete file)

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

https://w3c.github.io/webappsec/specs/subresourceintegrity/
Assignee: nobody → francois
Status: NEW → ASSIGNED
Attached patch Add integrity attribute (obsolete) — Splinter Review
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+
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
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
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
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 → ---
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+
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+
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
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.