Add rule to check for HTTP headers that are not needed for non-HTML resources #91
Conversation
| create(context: RuleContext): IRule { | ||
|
|
||
| const disallowedHeadersForNonHTMLResources = [ | ||
| 'content-security-policy', |
There was a problem hiding this comment.
Could there be other headers that we don't want for non html resources? Should we allow a configuration for this rule to allow ignoring some of those? Something similar to what you did for the other headers rule. Even if we don't agree, I think we should allow users to modify the headers to check or ignore.
There was a problem hiding this comment.
Could there be other headers that we don't want for non html resources?
That may be, and if not, there will probably be new ones in the future.
Should we allow a configuration for this rule to allow ignoring some of those?
As opposed to the other rule, my thinking was that this one is very specific, either you want to use it or not. Also, new additions should be discussed and added in this project so that more people can benefit.
Even if we don't agree, I think we should allow users to modify the headers to check or ignore.
Given it a second thought today, maybe you're right. One case in particular where I can see a need to configure this is when someone wants to progressively work on removing the headers, but cannot do it all at once (e.g.: can only remove the ones set at the application level and not the server level).
There was a problem hiding this comment.
allow users to modify the headers to check or ignore
Done.
Note: I also refactored some of the code.
| export const getRuleName = (dirname: string) => { | ||
| return path.basename(dirname); | ||
| }; | ||
|
|
||
| export const mergeIgnoreIncludeArrays = (originalArray: Array<string> = [], ignoreArray: Array<string> = [], includeArray: Array<string> = []) => { |
There was a problem hiding this comment.
@molant Didn't have much inspiration today, so feel free to suggest other names for the new function from this file.
No description provided.