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

Fixed Auto-upgrade #222

Closed
wants to merge 0 commits into from
Closed

Fixed Auto-upgrade #222

wants to merge 0 commits into from

Conversation

harinee
Copy link
Collaborator

@harinee harinee commented Aug 10, 2020

The regex looking for the latest tag was looking for string 'Location', whereas the actual string is 'location'.
So, changed the regex to be case insensitive. Also, appended a semi-colon in the string to reduce the scope of search.
Fixes #221

@dcRUSTy
Copy link
Collaborator

dcRUSTy commented Aug 10, 2020

https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location
After looking at these two... the way Github has changed case of header name... I can predict next bug it will introduce is adding some random header along with location may be some thing like ****-Location or ***location which will require repatching this regex again... How about adding a ^ sign or something better in the regex? P.S. Not a expert in regex

@dineshba
Copy link
Collaborator

@harinee As @dcRUSTy suggested we can add ^location: when we do grep. As the response code for curl -I https://github.com/thoughtworks/talisman/releases/latest is 302 we will always get location in the response as per https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/302

And also it is good that we are doing -i while searching for location. Because http header field names are case-insensitive. https://stackoverflow.com/questions/5258977/are-http-headers-case-sensitive

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto Upgrade is not working
4 participants