8000 fix: accept any Content-Disposition type when reading the filename by sandorkertesz · Pull Request #50 · ecmwf/multiurl · GitHub
[go: up one dir, main page]

Skip to content

fix: accept any Content-Disposition type when reading the filename - #50

Merged
iainrussell merged 1 commit into
developfrom
fix/assert_attachments
Nov 10, 2026
Merged

iainrussell merged 1 commit into
developfrom
fix/assert_attachments

Conversation

@sandorkertesz
Copy link
Copy Markdown
Contributor

Description

Fixes #49

HTTPDownloaderBase.extension() asserted that the Content-Disposition header was 'attachment'. Servers such as ECPDS (diss.ecmwf.int) send 'inline; filename=...', which RFC 6266 allows, so downloads failed with an AssertionError. The filename is now used whatever the disposition type, as title() already does.

Add offline tests for inline, attachment and custom disposition types, and a local HTTP server test.

Refs ecmwf/earthkit-data#1205

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

HTTPDownloaderBase.extension() asserted that the Content-Disposition header was 'attachment'. Servers such as ECPDS (diss.ecmwf.int) send 'inline; filename=...', which RFC 6266 allows, so downloads failed with an AssertionError. The filename is now used whatever the disposition type, as title() already does.

Add offline tests for inline, attachment and custom disposition types, and a local HTTP server test.

Refs ecmwf/earthkit-data#1205
@iainrussell iainrussell left a comment
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've had a look and am happy with this fix

@EddyCMWF EddyCMWF left a comment
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This assertion has been there since the start. As far as I can tell, if it is a problem it will just manifest in a different exception type for the downstream user elsewhere in the multiurl codebase.

I have checked cads-adaptors and ecmwf-datastores-client, and I believe they are not effected.

@iainrussell
iainrussell merged commit acc639c into develop Nov 10, 2026
114 of 141 checks passed
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.

Bug when getting data from diss.ecmwf.int/ecpds

3 participants

0