Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Cors headers are not properly set. #75

Open
AdriVanHoudt opened this issue Jan 21, 2020 · 0 comments
Open

Cors headers are not properly set. #75

AdriVanHoudt opened this issue Jan 21, 2020 · 0 comments

Comments

@AdriVanHoudt
Copy link
Contributor

AdriVanHoudt commented Jan 21, 2020

The problem is that atm we take over response handling (by returning h.abandon) which also means headers handling.
Doing cors handling ourselves would be a bit much, it would be nicer if we could just let hapi do that.

The proposed solution is to properly return/handle the transform streams we use back to hapi and then continue the lifecycle by returning h.continue.

For csv (works):

  • Set csvStream._readableState.objectMode = false; and don't pipe res
    • Quoting @kanongil (who helped a lot with these initial findings 🙏)

    It works because the implementation doesn't actually use objectMode for the output, and this is a way to change the mode for only that end

  • Set the headers on a headers property of the csvStream so that hapi will pick them up
  • Return csvStream

For Excel (not tested):

  • Pass a duplex stream to WorkbookWriter that has its readable side not in object mode
  • Set the headers on a headers property of the duplex stream so that hapi will pick them up
  • Pass the duplex stream to hapi

There is branch with tests and a working csv version here: https://github.com/Salesflare/hapi-csv/tree/cors-and-lifecycle-fix

AdriVanHoudt added a commit that referenced this issue Jan 21, 2020
And add a working version for csv, although it is kinda hacky-ish.

The whole issue is explained here: #75
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

1 participant