Crates.io: When ammonia supports allowing only certain values for attributes, re-enable syntax highlighting

Created on 24 Aug 2017  ยท  5Comments  ยท  Source: rust-lang/crates.io

We had to disable classes on code tags because you could use arbitrary classes to do arbitrary things. So for now, syntax highlighting in readmes is disabled.

I've filed an issue with Ammonia to request being able to specify allowed values for attributes (or at least classes); these are the classes that marky-markdown (used by npm) allows for their README rendering.

Instructions for implementing:

  • We've already updated to ammonia 0.7.0, which now has support for this!
  • [ ] Add the allowed_classes field to the ammonia configuration struct with a hash map that contains the tag name "code" and a value of a HashSet containing all the languages we want to support highlighting prefixed by language-, so language-bash, language-clike, etc.
  • [ ] Add a test in here like these tests that calls markdown_to_html with a code block that should get syntax highlighting, and assert that the code tag has class="language-whatever"
  • [ ] Add another test in there that tries to add classes that aren't allowed to a code element, or other classes to other elements, and assert that they aren't in the rendered HTML
  • BONUS: I'm happy to test this for real, but if you're feeling ambitious, publish a crate that has a README with some code in it to your local crates.io instance with the code change (instructions on publishing to your own instance are here) and make sure it is highlighted.
A-readme C-bug E-easy E-mentor T-infra WG-infra-crates.io

Most helpful comment

Thanks @carols10cents ! Okay, sounds like we can rule out S3.

I didn't ever setup a local bucket and can confirm that when running cargo run --bin server, I receive the local upload message.

Using local uploader, crate files will be in the local_uploads directory

Also, interestingly I don't have the readmes directory within local_uploads. My directory structure looks like this (run from root of crates.io directory):

โ†’ tree local_uploads                                                                                                                                                                                                                                                                                                [dc108ff]
local_uploads
โ””โ”€โ”€ crates
    โ””โ”€โ”€ hello
        โ””โ”€โ”€ hello-0.1.0.crate

2 directories, 1 file

So based on that, I guess we're looking into the readme HTML not getting created/uploaded when I published the crate. I can experiment bumping the version and republishing while adding some debugging statements within the related rendering and uploading functions.

I'll have another look at this tonight and see how I make out, if i'm still having issues then I'd say setting up a screen share would be great! Thanks for your help!

EDIT

Got it! It helps if you add readme = "README.md" within the Cargo.toml ๐Ÿคฆโ€โ™‚๏ธ , should have a PR up tonight.

All 5 comments

Ammonia supports it now.

I'd like to give this a shot. I'll let you know if I have any questions, thanks!

Hi @carols10cents, quick question on this. I've pretty much wrapped up all the checklist items and had published a create locally with some syntax highlighting. That said, when I view the crate details the README.md I created is not displayed and 404's.

The URL it's trying to reach is http://localhost:4200/readmes/hello/hello-0.1.0.html (hello is the name of my test crate obviously), and the server response is 404 with {"errors":[{"detail":"Not Found"}]}

Any thought's as to why the readme won't render? I'm suspecting possibly something with S3?

Hi @treiff! Have you set up an s3 bucket locally, and put S3 config vars in your .env? If not, the files should be "uploaded" to your crates.io directory and served from there. When you run cargo run --bin server, do you see this?

$ cargo run --bin server
    Finished dev [unoptimized + debuginfo] target(s) in 0.2 secs
     Running `target/debug/server`
Using local uploader, crate files will be in the local_uploads directory

If so, that means we can rule out s3 being the problem :)

Where you have crates.io's code checked out, can you look in local_uploads/readmes? Is there a directory in there for hello, and is there a hello-0.1.0.html file in there?

If that file does exist, then for some reason the server isn't seeing that. We can poke around in https://github.com/rust-lang/crates.io/blob/264dd1a53d5055693a44654aad006cb6cf68bf7f/src/local_upload.rs to see where it might be going wrong.

If that file doesn't exist, then the readme HTML isn't getting created/"uploaded" on crate publish :( The rendering happens here, which calls this markdown_to_html function. The "uploading" happens here which calls this upload method.

We'd need to try publishing more crate versions and narrowing down where that might be failing.

Let me know if you'd like to jump on a screensharing session or something to figure this out in real time!

Thanks @carols10cents ! Okay, sounds like we can rule out S3.

I didn't ever setup a local bucket and can confirm that when running cargo run --bin server, I receive the local upload message.

Using local uploader, crate files will be in the local_uploads directory

Also, interestingly I don't have the readmes directory within local_uploads. My directory structure looks like this (run from root of crates.io directory):

โ†’ tree local_uploads                                                                                                                                                                                                                                                                                                [dc108ff]
local_uploads
โ””โ”€โ”€ crates
    โ””โ”€โ”€ hello
        โ””โ”€โ”€ hello-0.1.0.crate

2 directories, 1 file

So based on that, I guess we're looking into the readme HTML not getting created/uploaded when I published the crate. I can experiment bumping the version and republishing while adding some debugging statements within the related rendering and uploading functions.

I'll have another look at this tonight and see how I make out, if i'm still having issues then I'd say setting up a screen share would be great! Thanks for your help!

EDIT

Got it! It helps if you add readme = "README.md" within the Cargo.toml ๐Ÿคฆโ€โ™‚๏ธ , should have a PR up tonight.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sfackler picture sfackler  ยท  9Comments

carols10cents picture carols10cents  ยท  7Comments

carols10cents picture carols10cents  ยท  8Comments

ehuss picture ehuss  ยท  5Comments

dtolnay picture dtolnay  ยท  6Comments