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

WIP #203 Fixing issue with mixed content #211

Closed

Conversation

AdrianAcala
Copy link

This is to fix issue #203.

@trafico-bot trafico-bot bot added the 🚧 WIP Still work-in-progress, please don't review and don't merge label Jun 4, 2023
@AdrianAcala AdrianAcala changed the title WIP #203 Fixing issue with mixed content #203 Fixing issue with mixed content Jun 4, 2023
@trafico-bot trafico-bot bot added 🔍 Ready for Review Pull Request is not reviewed yet and removed 🚧 WIP Still work-in-progress, please don't review and don't merge labels Jun 4, 2023
@j-f1
Copy link
Contributor

j-f1 commented Jun 5, 2023

Thanks for opening this PR @AdrianAcala! Unfortunately, I tested locally and this doesn’t seem to work. Specifically, I generated an HTTPS certificate and key, then configured config.json to enable HTTPS using them. Next, I started the server and made a request to http://localhost:5006. That request gave me a “socket closed by remote peer” error when requested from a browser, and curl: (52) Empty reply from server from curl.

@AdrianAcala AdrianAcala changed the title #203 Fixing issue with mixed content WIP #203 Fixing issue with mixed content Jun 5, 2023
@trafico-bot trafico-bot bot added 🚧 WIP Still work-in-progress, please don't review and don't merge and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Jun 5, 2023
@P1514
Copy link

P1514 commented Jun 6, 2023

Yes your approach was the first one I though. Issue is req never gets into that because port is configured as https not http. Failure is on protocol level not on code level.
I think the best approach would be a proxy pass with https leaving actual with http only. But that's outside of scope of the project I think

@AdrianAcala AdrianAcala force-pushed the https-mixed-content-fix branch 3 times, most recently from 9687e04 to 0b41950 Compare June 6, 2023 18:18
@AdrianAcala
Copy link
Author

AdrianAcala commented Jun 6, 2023

@P1514 , my research shows that you can have two concurrent express apps running at the same time so the solution seems to be to have two apps listening on both ports. I've made the appropriate changes. Can you give it another shot, please?

@j-f1
Copy link
Contributor

j-f1 commented Jun 6, 2023

It looks like you’re now opening port 80. I don‘t think this will work if you’re running on a machine that’s already listening on port 80 for something unrelated, and with a container, only port 5006 would continue to be exposed. Additionally, http://<actual-ip>:5006 would continue to error out since that port only listens for HTTPS connections.

@AdrianAcala
Copy link
Author

@j-f1 , if you have something else listening on port 80, like Nginx, you should redirect or reverse proxy using the web server. You can't expect this application to handle that redirect for you.

Closing this PR as it is no longer necessary.

@AdrianAcala AdrianAcala closed this Jun 6, 2023
@j-f1
Copy link
Contributor

j-f1 commented Jun 6, 2023

Thanks for giving this fix a shot!

@AdrianAcala AdrianAcala deleted the https-mixed-content-fix branch January 30, 2024 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚧 WIP Still work-in-progress, please don't review and don't merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants