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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(http) highlight http headers without HTTP preamble #3019

Closed
marcoscaceres opened this issue Feb 22, 2021 · 7 comments · Fixed by #3051
Closed

(http) highlight http headers without HTTP preamble #3019

marcoscaceres opened this issue Feb 22, 2021 · 7 comments · Fixed by #3051
Labels
bug help welcome Could use help from community language

Comments

@marcoscaceres
Copy link
Contributor

Describe the issue
It should be possible to declare http key/value pairs without requiring HTTP preamble.

For example, just:

Some-header: value

Which language seems to have the issue?
HTTP

Are you using highlight or highlightAuto?
highlightAuto

...

Sample Code to Reproduce

Referer: value

Expected behavior

Should output:

<span class="hljs-attribute">Referer</span>

Additional context
Appears to be a regression from 10.5. It used to work. Maybe #2893?

@marcoscaceres marcoscaceres added bug help welcome Could use help from community language labels Feb 22, 2021
@joshgoebel
Copy link
Member

joshgoebel commented Feb 22, 2021

Maybe #2893?

Yes, that would be it.

Not sure what the easy fix for this is though. subLanguage: [*] is very scary for auto-detection and the recursive nature of this usage in particular could potentially be used against us in some form of attack. Requiring the preamble is much safer. If you have your own install you could probably restore the old behavior by hacking the JS grammar manually immediately after loading (just OTTOMH):

<script src="highlight.min.js" />
<script>
let http = hljs.getLanguage("http");
// move HEADERS_AND_BODY back to the parent ruleset
http.contains = [].concat(http.contains, http.contains[0].starts.contains)
</script>

Again, not sure that is a good idea though (depending on use case).

I'd also perhaps be OK with disabling the inner * auto-detection (and just not highlighting HTTP content itself) - but I think what we're doing now is likely a better tradeoff for MOST users.

One other possibility might be to change subLanguage: * to disallow going recursive, but I'm not sure that's as simple as it sounds (and not sure if it would break any other grammars).

@joshgoebel joshgoebel changed the title (http) regression highlighting http attributes without HTTP preamble (http) highlight http headers without HTTP preamble Feb 22, 2021
@marcoscaceres
Copy link
Contributor Author

Is there some cheap/cheat way we could support this if the language is explicitly provided? I.e, if I'm sure it's http and I go through highlight()?

@joshgoebel
Copy link
Member

joshgoebel commented Feb 24, 2021

I'm not sure how off the top of my head... there is truly ONLY highlight... highlightAuto merely wraps highlight in a loop... I'm perhaps open to ideas here (as I can see it being useful sometimes) but it would also slow things down by requiring two highlighting passes of a winning language... (or at least in cases where we know the behavior might be different). IE, if you auto-detected HTTP you'd have then run HTTP again to get the FINAL output.

And I think you'd still want to solve the sublanguage recursion issue.

@joshgoebel
Copy link
Member

joshgoebel commented Mar 17, 2021

I think it might be ok if we added a HEADER rule to the top-level (with 0 relevance because it's too conflicting with auto-detect)... so then you'd have two choices:

  • a FULL http request (with the body parsed) keyed off of using the preamble
  • Just HTTP headers, nothing more (and auto-detect wouldn't "see" them)

This might add a bit more utility while also creating a bit more confusion.


IE take line 15 and pull that out into a HEADER rule that matches at the top. Thoughts?

@marcoscaceres
Copy link
Contributor Author

That sounds reasonable... if one wants autodetect to work, then one must include the preamble. Otherwise, one must be explicit that it's http.

@joshgoebel
Copy link
Member

joshgoebel commented Mar 17, 2021

Well the potential point of confusion is that HEADERS + BODY will NOT work (regardless - body won't get highlighted, only headers)... that requires the preamble. The preamble is the guard that triggers "full" support.

IE, Body highlighting is triggered only by the preamble.

@marcoscaceres
Copy link
Contributor Author

ah, got it. I think that seems reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help welcome Could use help from community language
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants