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

Server should check whether header has already been set to prevent overwriting #48

Closed
9662 opened this issue Jun 4, 2018 · 6 comments · Fixed by #49
Closed

Server should check whether header has already been set to prevent overwriting #48

9662 opened this issue Jun 4, 2018 · 6 comments · Fixed by #49
Labels

Comments

@9662
Copy link
Contributor

9662 commented Jun 4, 2018

This line:

res.setHeader('Content-Type', extname ? mime.lookup(extname) : 'application/octet-stream');

interferes with my own middleware, which sets Content-Type for files that are not recognised by mime.lookup, but then the header gets overwritten by the above line.

The router middleware should check if the header has already been set before trying to set its value.

@tcrowe
Copy link

tcrowe commented Sep 24, 2018

Hey @9662 👋 How did you run into that problem? Can you provide a test case?

@9662
Copy link
Contributor Author

9662 commented Oct 5, 2018

I am afraid I no longer recall the circumstances around this issue, but looking at my activity around the reporting date, I think a realistic test case could be as follows:

Have a generator middleware that extracts event-related custom metadata from the front matter and uses it to produce .ics files. Because the generated ICS files would normally be served by hexo serve with an inadequate Content-Type (possibly application/octet-stream or text/plain) they would not trigger the desired browser behaviour while testing locally, therefore you will also be needing a filter middleware to add the correct Content-Type: text/calendar header. This filter middleware would cause the reported error.

In the end I must have implemented this differently so I never followed up.

@tcrowe
Copy link

tcrowe commented Oct 5, 2018

@9662 Oh if there was a solution maybe you can post it. How do you want to proceed? Should we still look for a solution or close it?

@9662
Copy link
Contributor Author

9662 commented Oct 6, 2018

@tcrowe Seems like a had an old pull request dealing with this. Let me check if it's salvageable.

@tcrowe
Copy link

tcrowe commented Oct 6, 2018

#49

@9662
Copy link
Contributor Author

9662 commented Oct 6, 2018

Yup, that's the one. I just rebased and updated.

@stevenjoezhang stevenjoezhang linked a pull request May 18, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants