Skip to content

add Accept header when downloading file#2437

Merged
boegel merged 2 commits intoeasybuilders:developfrom
oliviermattelaer:patch-1
Mar 19, 2018
Merged

add Accept header when downloading file#2437
boegel merged 2 commits intoeasybuilders:developfrom
oliviermattelaer:patch-1

Conversation

@oliviermattelaer
Copy link
Copy Markdown
Contributor

when trying to download file with urllib2 adding to the header the field Accept since the absence of such field can lead to some security module to raise a 403 response:
https://serverfault.com/questions/377656/why-does-mod-security-require-an-accept-http-header-field

when trying to download file with urllib2 adding to the header the field Accept since the absence of such field can lead to some security module to raise a 403 response
Comment thread easybuild/tools/filetools.py Outdated

# use custom HTTP header
url_req = urllib2.Request(url, headers={'User-Agent': 'EasyBuild'})
url_req = urllib2.Request(url, headers={'User-Agent': 'EasyBuild', "Accept" : "text/html"})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oliviermattelaer What does the text/html part mean here exactly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, The only part check by the apache module seems to be that "Accept" is in the header and not empty.

But indeed it might make more sense to use here "/" which does not imply any restriction on the type of content that we accept.

Cheers,

Olivier

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use */* rather than text/html, to accept all types... Cfr. https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html

Please make that change, then it should be OK to merge this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Done.

Thanks,

Olivier

@boegel boegel added this to the 3.6.0 milestone Mar 13, 2018
@boegel boegel changed the base branch from master to develop March 19, 2018 08:57
@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 19, 2018

@oliviermattelaer For future pull requests, please target the develop branch (I've changed this myself for this PR).

Going in, thanks for your contribution!

@boegel boegel merged commit 7db62c1 into easybuilders:develop Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants