-
Notifications
You must be signed in to change notification settings - Fork 2
match new hash pattern, log when replacing URI paths #595
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
base: master
Are you sure you want to change the base?
Conversation
zap/src/scan.py
Outdated
r=r._replace(path=URI_HASH_REGEX.sub('', r.path)) | ||
r_prev = r | ||
r=r._replace(path=URI_HASH_REGEX1.sub('', r.path)) | ||
r=r._replace(path=URI_HASH_REGEX2.sub('index-hash-.js', r.path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the only problem index.js?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only one I saw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry to say I have pushed a change here to remove the hash as discussed in the meeting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about the timing but not the change.
I put the replacement strings together with the regexes. Because URI_HASH_REGEX2 always matches "index-XXXXXXX-.js" it gets replaced with "index.js", so it should be consistent with how the previous regex works -- just removing the hash.
I'm sorry to have combined a reformatting of this code with the real change which was around URI_HASH_REGEX |
Now it only removes the hash. I put the replacement strings together with the regexes so it's easier to follow what happens: Because URI_HASH_REGEX2 always matches "index-XXXXXXX-.js" it gets replaced with "index.js", so it should be consistent with how the previous regex works -- just removing the hash. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. There's definitely some future changed we can make to the regex to make it a bit more precise, but I think this is a good addition.
Paths look like this now:
/assets/index-4au49BA-.js
So run an extra regex to handle paths like that.
Also add a log message whenever a path is replaced via regex.