Highlight.js: Version 9 EOL & End of Support (even for security issues)

[joshgoebel]

A recent vulnerability has brought the topic of version 9 support back into the forefront. The core team doesn't really have the time or focus to maintain two very different forks of the project. Especially since the version 9 only truly exits to service a tiny audience (IE11, <1% browser share by caniuse.com). We were kind of OK letting it live a bit longer as long as it didn't require any time/effort from us, but that's no longer proving to be the case.

• Many, many issues and bugs have been fixed since the v10 release.
• Many, many new features have been added.
• Many grammars highlight WAY better than they did previously.
• The v10 parsing engine is so much more flexible and reliable.
• There are quite likely numerous vulnerabilities we've fixed along the way without even realizing it.

It's so obvious that v10 and modern browsers (released in the last ~4-5 years) is where we need to be focusing our time, not supporting very old browsers. So if you're still using version 9:

If you aren't supporting IE11 users (or others with very obscure browsers)

You need to upgrade to v10. No question. For 90% of simple cases it's a trivial upgrade. For complex integrations it may be a bit more effort (like most things). https://github.com/highlightjs/highlight.js/blob/master/VERSION_10_UPGRADE.md

If you're supporting IE11 users (or others with very obscure browsers)

Someone truly requiring IE11 support for "enterprisey" projects should perhaps look at Prism.js, which is a great project that still supports IE11. Or perhaps consider maintaining their own private fork of version 10 that supports IE11. This should still technically be possible now (AFAIK) but may prove more difficult as time goes on.

So here is the current plan:

• [x] One last v9 release (security fix).
• [x] That release's release notes will mention the EOL and link to this issue. (and perhaps spew some other output, need to look at what is possible with npm).
• [ ] That release itself (and the current v9 release) will both be marked deprecated with a message to upgrade to v10.
• [x] We may mark other prior releases deprecated as well. _I'm really curious why more npm projects don't do this. Does anyone know?_
• [x] SECURITY.md will be updated accordingly.
• [x] v9 support will then be official EOLed.

So v9 users would want to upgrade to the latest v9 release and then either start working on upgrading to v10 or come up with other plans.

[TimotheDavid]

hello I try to upgrade highlight with npm in node and typeorm but I don't find the folder ... and you don't explain ho to do .... could you explain ?

thanks !

[TimotheDavid]

could you remove the warning when ou install highlight 10.4 : I have already the warning for 9 version .... thanks a lot !

[joshgoebel]

There is no warning on the new version.

[Romakita]

Hello @joshgoebel
Which new version? Because the warning still occur on 9.18.4 and it's a bit aggressive. Especially if the warning comes from a transitive dependency. Can you remove it or change the warning by a simple (and small) depreciation warning please.

Deprecation must be instrumented with npm deprecate: https://docs.npmjs.com/cli/v6/commands/npm-deprecate

Thanks :)

[joshgoebel]

The warning should only be on post-installation.

Because the warning still occur on 9.18.4

Yes, the entire 9 series is no longer supported. We pushed one last update to 9 just so anyone still using 9 would have a window to work on upgrading to 10. It's not intended that anyone use 9.18.14 long-term. The intention is that everyone upgrades to v10.

Deprecation must be instrumented with npm deprecate

Yes, we're deprecating them also. I'm not sure anyone pays attention to those though. I certainly haven't in the past.

[Romakita]

This is exactly the problem. My users install the cli and have an unexpected warning in full page, for a dependency that does not concern them (nor me for that matter). I can fix this problem because the helper-markdown maintainer is responsible to update his code.

I think the right solution is to use npm deprecate instead of a postinstall script!! (it's really a bad practice). By using npm deprecate, you inform npm and many other tools like snyk about the depreciation. Your postinstall script has no benefit for the final user ^^.

See you
Romain

[joshgoebel]

First time doing something like this on NPM. :-) Yes, I understand the issue you're raising now. We need to think about what message we intend to send to users where we are not a direct dependency. Which we didn't take into account (though it seems obvious in hindsight). Are there specific guidelines published somewhere that specifically say "don't do this" with post-install?

It's definitely annoying that the "end user" here doesn't have a _direct_ course of action, but I assume they would bug you, then you would bug upstream, etc... The intended message was supposed to be more of "you REALLY need to upgrade" vs "hey this is old now". I feel like maybe there is no great choice here (or I just don't know best practices). I feel deprecation alone is too quiet and it's possible post-install is too noisy...

I'll talk it over with the team. I assume at this point we'd have to push out yet another release just to remove the warning, right? :-|

[Romakita]

Are there specific guidelines published somewhere that specifically say "don't do this" with post-install?
Not especially, it's true. It's clearly my point of view, and we can agree on the fact that the big warning in post install is a bad use of postinstall ^^

I'll talk it over with the team. I assume at this point we'd have to push out yet another release just to remove the warning, right? :-|
I'm you can take a time to find a proper way to instrument that or just change the message so that it's less intrusive ;)
I understand your point of view though
Thanks

[joshgoebel]

I'm you can take a time to find a proper way to instrument that

Did you have a suggestion other than deprecate?

or just change the message so that it's less intrusive ;)

What level of intrusion would you find reasonable? (just curious) :-)

[ZQun]

Hello, the warning message when prompted for this version upgrade has some impact on the visual effect. It will prompt the version upgrade every time you save the file. Because we rely on other libraries, we don’t have the permission to update, but the upgrade prompt message will be updated every time during our development process. It will prompt you every time you save the file, which is very unfriendly!

[ZQun]

@joshgoebel
Hello,
The code here will cause us to prompt update information every time we save a file during program development, bad experience!
https://github.com/highlightjs/highlight.js/blob/c34318b6a720a0852d27cd13dc55ca896e1292ec/src/highlight.js#L33

[Romakita]

What level of intrusion would you find reasonable? (just curious) :-)

Only one or two lines with the link is enough, not a big banner. This is the responsibility of each maintainer to upgrade his dependencies. Your point is just to said to theses maintainers to upgrade his dependencies. If the maintainer doesn't have a time or doesn't want upgrade it, isn't not your responsibility ;). Your work is accomplished as long as you have indicated that version 9 is deprecated and that you will no longer support it for many very justified reasons (and posted on your github project).

My point is, because you've added this banner, it has a direct impact on my CLI installation (and user experience), when any developer install/use it on his system. It's not something that I wanted and I don't want to have feedback on a subject that is not my scope either ;)

Did you have a suggestion other than deprecate?

npm deprecated is the standard for your problem (or I forgot something). So I haven't other solution.

[ManuGM91]

I would also like to echo what @Romakita has said. I think your job would be done, when you put out a warning saying this isn't supported any more and you have to go for new version. It is completely up to the maintainers to upgrade or not, their dependencies according to their requirements. Any issues they would face in future due to not upgrading to the latest version should be completely owned by them.

If you see the two bugs(one raised by me) where this issue has been mentioned, I believe both of them have no direct connection with what is currently there, but the impact is a bit big.

[joshgoebel]

@ZQun Probably helpful to discuss the warn separately from the post-install notices.

It will prompt the version upgrade every time you save the file.

That's a bit unusual... why are you running npm install every time a file is saved?

[joshgoebel]

I think your job would be done, when you put out a warning saying this isn't supported any more and you have to go for new version.

That's what I thought I was doing. I don't consider deprecate a "warning". The warning is visual (for sure) but if it actually BREAKS someones build I don't understand why... taht's what I'd be very curious to understand before we decide what to do here.

[joshgoebel]

@MGM19 I saw your single issue, is there two?

[ZQun]

@ZQun Probably helpful to discuss the warn separately from the post-install notices.

It will prompt the version upgrade every time you save the file.

That's a bit unusual... why are you running npm install every time a file is saved?

@joshgoebel
Hello,
You see, after I run the program in npm run dev, I will prompt every time I save a file

[joshgoebel]

Ah, I understand what you mean now. Yeah, it is a bit more annoying if you're restarting your whole application every time a single file is saved.

[ZQun]

Ah, I understand what you mean now. Yeah, it is a bit more annoying if you're restarting your whole application every time a single file is saved.

@joshgoebel
Hope you can handle it, thank you!

[joshgoebel]

@ZQun Well, it's not causing any harm other than being a little noisy... in development you could simply comment out the warning line in highlight.js and then it would shut up...

I'm still trying to figure out how we want to approach this big-picture.

[ManuGM91]

@joshgoebel
Sorry, if I put that out wrong, but I was meaning that total of two bugs - one from me and the other from @ZQun .

For me it's not the visual impact to be honest. I am using https://github.com/DannyDainton/newman-reporter-htmlextra to generate html reports for our API testing using Postman and Newman. The reporter have a dependency with highlight. So now I have a step in my Pipeline in which I use powershell script to run my collection and generate a report using the reporter. The step is now failing with an ##[error] badge. Since this step decides whether my pipeline should pass or fail, I can't suppress this error.

[joshgoebel]

@MGM19 Yes, I see your issue now. Just to be clear though we are NOT raising (or logging) any kind of error, only a warning. Your build system is choosing to interpret the warning as an error.

[ManuGM91]

in development you could simply comment out the warning line in highlight.js and then it would shut up...
My situation is that I can't even take this approach. All our pipelines are failing due to this and I am unable to explain a valid reason why it's happening to many, since highlight doesn't have any direct dependency.

[joshgoebel]

@MGM19 You can always re-pin 9.18.3 until this is resolved to your satisfaction. The library (both v9 and v10) logs warnings for other reasons though as well, so I'd consider looking into why your build system is treating a warning as a hard error.

So now I have a step in my Pipeline in which I use powershell script to run my collection and generate a report using the reporter. The step is now failing with an ##[error] badge.

What criteria is Powershell using to decide an "error" has occured? Just any output at all? I confirmed locally that using console.warn does not alter the error status returned when executing node.

[ManuGM91]

To be Honest, I am not sure if I can answer the criteria question. I haven't specified any criteria. The Pipeline is being configure using YAML and the specific step looks something like this.

• pwsh: |
  newman run "${{ parameters.collectionName }}" -e "${{ parameters.environmentFileName }}" --reporters "cli,htmlextra" --reporter-htmlextra-template ${{ parameters.htmlextraVersionFileName }}
  displayName: 'run newman tests'
  enabled: true
  condition: succeeded()
  failOnStderr: true
  workingDirectory: '${{ parameters.workingDirectory }}'

[ManuGM91]

I got your point that, your's is more than a warning rather than an error. But, I am not sure why powershell is considering it as a error.

[joshgoebel]

https://nodejs.org/api/console.html#console_console_warn_data_args

The console.warn() function is an alias for console.error().

Gah, cause Node is so dumb. :-) Even another very smart associate I asked just now got this wrong also. Warnings are not errors... but if you ask Node evidently they are (and they log to stderr). So we'll probably have to do something about it now I think (and probably change our library in other places also long-term [at least when running in Node]).

But if you're broke, broke atm, please revert to 9.18.3 until we get this sorted.

[Romakita]

@joshgoebel
I given a demo today with the CLI. You can see, the log appear each time:

Is clearly not normal ^^.

[ManuGM91]

The problem @joshgoebel currently is that, I believe I have very little/no control over upgrading/reverting highlight.js and it's more with the maintainers of the reporter.
I may be wrong, but as I am not an expert with node or npm, I am not sure how to head forward?
Any help would be really appreciated. With my limited knowledge, I believe I have to wait until this is resolved.

[ManuGM91]

By the way, Thank you and happy that you found the root cause of why this happening. Really helpful is that bit of info.

[joshgoebel]

@MGM19 In your own project can you not pin the version yourself (making it a direct dependency just to pin it):

npm install [email protected]

Really helpful is that bit of info.

Would have been really helpful yesterday. :)

[Romakita]

@joshgoebel This is a small fix, but suddenly, we find ourselves adding a dependency that is not ours directly :)

[joshgoebel]

@Romakita I didn't say it was a permanent solution, but MGM19 was asking how he could take some control.

[Romakita]

yes :) no worries.

[ManuGM91]

I have tried that and not sure why, that's not solving the issue. Still I can see the error. When I checked package.json inside the highlights.js folder inside the reporter's node_modules folder I am seeing this.

[Romakita]

have you tried npm dedup after using the npm install ?

[joshgoebel]

You can't change the reporters package version, but I thought if you installed it yourself that the reporter might use that version when invoked from your package - I'm not sure how such things work with NPM honestly.

[ManuGM91]

have you tried npm dedup after using the npm install ?

Tried this now, but didn't help unfortunately.

[ManuGM91]

You can't change the reporters package version, but I thought if you installed it yourself that the reporter might use that version when invoked from your package - I'm not sure how such things work with NPM honestly.

I too had a similar thought, but it didn't seems to be helping much. TBH, I am also not so aware how the upgardes work within NPM.

[joshgoebel]

I plan to push a 9.18.5 release in a bit to resolve the warn vs log issue (and give people a way to turn off the warning). Of course if your dependency has upgraded to 9.18.4 then they would also need them to upgrade to 9.18.5 to resolve your issue with warn logging to STDERR.

[Romakita]

I plan to push a 9.18.5 release in a bit to resolve the warn vs log issue (and give people a way to turn off the warning). Of course if your dependency has upgraded to 9.18.4 then they would also need them to upgrade to 9.18.5 to resolve your issue with warn logging to STDERR.

Not necessarily. Example here: https://github.com/helpers/helper-markdown/blob/master/package.json#L29
The maintainers allow all versions after "^9.12.0", it doesn't need to update the dependency, but it explain why your banner have a side effect on any projects which use hightlight.js today.

[ManuGM91]

Apologies @Romakita if this is a stupid question. So you meant to say when @joshgoebel pushes 9.18.5 it should, by default fix the issue and there is no upgrade we have to do even if it's one of our dependency?

[Romakita]

@MGM19 It should but regarding the PR, It won't solve really the problem.
@joshgoebel Has added options and env variable to shutdown the warning (at runtime) by the code. It's a good intention, but
if in our case, the code is probably executed by a transitive dependencies, which it doesn't really know the existence of this issue/options.

And the questions as maintainers:
why I should add an option or an environment variable in my code for a log problem which would come from a transitive dependency. If we allow that then, anyone can add an environment variable to disable unwanted logs.

[Romakita]

After explanation, the 9.18.5 should fix the problem if the transitive dependency don't call the method which contain the log. (but isn't sure).

@joshgoebel I'll give you feed when your release will be available. Can you consider, using this highlight method in Node.js context, the security issue is valid?

[joshgoebel]

The maintainers allow all versions after "^9.12.0",

@MGM19 If this is the case then you should indeed be able to fix it yourself now that 9.18.5 is released. The warn has been removed so your build system shouldn't see any error anymore. You still may need npm install highlight.js or similar to bump the dependency.

Can you consider, using this highlight method in Node.js context, the security issue is valid?

If Highlight.js is being used to highlight user provided content there are potential issues (whether in the browser or on the server). And again these are only the known unknowns. :-) Version 9 is really VERY VERY old an out-of-date.

from a transitive dependency.

It doesn't matter how deep a dependency it is, if you're using it for it's said purpose (even indirectly) then you should be aware of the deprecation and the security concerns of not upgrading. And if you aren't truly using it, then you shouldn't see the warning. We'll see how 9.18.5 goes. :)

If we allow that then, anyone can add an environment variable to disable unwanted logs.

I don't know what you mean here.

[joshgoebel]

@MGM19 Sorry about breaking your build. :) Lesson learned about warn. And I've created an issue to track it in version 10 and see what changes may need to be made:

https://github.com/highlightjs/highlight.js/issues/2880

[Romakita]

To be clearer if each npm module introduces an environment variable to disable logs that I did not have when developing my code, I will end up with a number of unwanted warnings and independent of my wish. I would therefore be forced to integrate these variables to fix the problem while everyone does the migration work. which may take time.

I don't want to deal with this complixity at the level of my code. Because it’ll be unmaintainable :)

we agree the best solution is to update but must it be possible.

[joshgoebel]

Again just to be clear: It's not just some random log output, it's a SERIOUS warning about version 9 deprecation and potential security concerns that people using the library (directly or indirectly) should know about. It now only shows up when the library is actively used. We provide 3 different paths to disable the warning. When it comes time to deprecate v10 we'll keep this bigger picture in mind and see if we can come up with something better.

We could have done better with 9.18.4 (hindsight is 20-20), but I'm pretty happy with 9.18.5. We'll see. :-)

[Romakita]

Yes sure ;) thanks again

[joshgoebel]

https://github.com/highlightjs/highlight.js/issues/2882

[ManuGM91]

@joshgoebel , I am happy to let you know that my issue is now resolved and I didn't have to do anything except uninstall the newman htmlextra reporter and install it again. I think @Romakita was correct with maintainers allowing all version after 9.12.0. Now my builds are back to green. I would really like to appreciate all your time and efforts and also like thank you for acting on it swiftly.

[ManuGM91]

@MGM19 Sorry about breaking your build. :) Lesson learned about warn. And I've created an issue to track it in version 10 and see what changes may need to be made:

2880

Please don't be sorry. We all have met these unforeseen situations and in my case to be honest, I think you haven't done anything wrong as it's npm which considers warning to hard error. In one way it's good that, we all have our lessons learned and personally I am happy that I got to interact with you guys.

Thanks again for fixing it and doing it swiftly. I have tried 3 different builds and all have gone green, so I would assume that it is resolved. Any way I will confirm by EOD about any unseen issues if anything happens.

[ManuGM91]

Just for information, the highlighted part in screenshot of package.json for highlight.js is what changed after I re-installed the reporter.

[ZQun]

@joshgoebel
Hello,
[email protected] did not install successfully, is it because I used the wrong command?

[joshgoebel]

That's weird. Try it again?

[ZQun]

That's weird. Try it again?

It feels like version 9.18.5 is in a different tab, "previous":"9.18.4","unsupported":"9.18.5"

After I executed npm install, I did not install 9.18.5 into the dependencies package-lock.json

[ZQun]

@joshgoebel
Hello,
After I execute npm install, the default installation is still 9.18.4

[ManuGM91]

@ZQun Is it possible for you to uninstall(may be globally) your package for which highlight.js is a dependency? And then install that again? As mentioned earlier, for me uninstalling my reporter and installing it again fixed the issue and it automatically picked 9.18.5.

[joshgoebel]

Yes they were tagged differently but if you specify the version directly that should work regardless of the tag. You could also try

npm install highlight.js@unsupported
npm uninstall highlight.js@previous

Or try @ManuGM91 suggestion.

[ZQun]

Thanks for answer,
There is no problem using npm source to install. I can’t install using npm source in China. I have found a way and execute npm install git+https://github.com/highlightjs/highlight.js.git#9.18.5 --save can be installed successfully!

update:

It is problematic to install dependencies through github. The contents of ./lib/* are missing in the directory. Now version 9.18.5 can be installed through the npm source in China. It is probably because the latest npm mirror has not been synchronized before, and the problem has been solved.

[joshgoebel]

@ZQun Our primary source repository is NOT a build product - it's only the raw source.

There is of course https://github.com/highlightjs/cdn-release, but the build is in build... I'm not sure if there is a way to tell npm to install from a repo using a given path. N/m CDN isn't going to help you because you're wanting the NPM library release I assume... yeah I dunno what to tell you if you can't actually access the NPM repository.

You could always build from source and then do the install locally.

[ZQun]

@ZQun Our primary source repository is NOT a build product - it's only the raw source.

There is of course https://github.com/highlightjs/cdn-release, but the build is in build... I'm not sure if there is a way to tell npm to install from a repo using a given path. N/m CDN isn't going to help you because you're wanting the NPM library release I assume... yeah I dunno what to tell you if you can't actually access the NPM repository.

You could always build from source and then do the install locally.

@joshgoebel
Thank you for your answer. The problem has been solved, mainly using typeorm. Today I found that the following dependencies have been upgraded to version 10.x!