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

The sensitiveHeaders HTTP2 symbol should be initialized via Symbol.for instead #36282

Open
szmarczak opened this issue Nov 26, 2020 · 2 comments
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@szmarczak
Copy link
Member

Is your feature request related to a problem? Please describe.

const kSensitiveHeaders = Symbol('nodejs.http2.sensitiveHeaders');

Currently to use the symbol we need to import it from the http2 module. A better solution would be to use Symbol.for like in the case of custom inspect symbol.

Describe the solution you'd like

-const kSensitiveHeaders = Symbol('nodejs.http2.sensitiveHeaders');
+const kSensitiveHeaders = Symbol.for('nodejs.http2.sensitiveHeaders');

Describe alternatives you've considered

None.

@silverwind silverwind added the http2 Issues or PRs related to the http2 subsystem. label Nov 27, 2020
@Lxxyx
Copy link
Member

Lxxyx commented Nov 27, 2020

In the documentation(https://github.com/nodejs/node/blob/master/doc/guides/using-symbols.md), there are references to how global Symbols are used.

Global symbols should be preferred when a developer-facing interface is needed to allow behavior customization, i.e., metaprogramming.

After trying to read the related Issue (#34091) and Pull Request(#34145).

I think that sensitiveHeaders is a feature that developers are using in the http2 module, not a developer-facing interface, and naming it nodejs.http2.sensitiveHeaders may be a mistake.

Details still need to be explained by @addaleax , if this is a mistake, I will file a Pull Request to fix it.

@szmarczak
Copy link
Member Author

Global symbols should be preferred when a developer-facing interface is needed to allow behavior customization, i.e., metaprogramming.

That's exactly the case here. If you declare the symbol in your headers object then the headers in that array won't be compressed. It's custom behavior, isn't it?

I think that sensitiveHeaders is a feature that developers are using in the http2 module, not a developer-facing interface

Following your thinking, let's revert nodejs.util.promisify.custom and nodejs.util.inspect.custom, because developers are using it in the util module.

A module is an interface. API stands for Application Programming Interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants