-
Notifications
You must be signed in to change notification settings - Fork 119
fix: reverse proxy url fix #1995
Conversation
|
Hint: As there are no other places which use _links from backend this change should be fine. If there are other occurrences the new function can be used there as well. |
|
Hey @claudiahub / @oodamien - what do you think about that fix? We recently tested it and it works behind reverse proxy and direct access without changes to the backend. |
claudiahub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@klopfdreh looks good to me, thank you
it seems there are no other places where we use _links from backend.
7feba18 to
953e6fb
Compare
|
Hey @claudiahub - @onobc just asked me if I could provide an opt-in setting for the reverse proxy fix which can be activate through the settings page. I force pushed the changes to this PR. Here is how the ui looks like, now: The default behavior is deactivated. Can you check if everything is working for you properly? |
953e6fb to
672f648
Compare
672f648 to
9f1b3ef
Compare
|
Thanks for the contribution @klopfdreh ! Closing via f0f0820 |

UI accessed over reverse-proxy:
http://reverse-proxy-url:443/scdf/Actual backend url:
http://somedomain.de:8080/scdf/If you access _links the actual backend is rendering those as
http://somedomain.de:8080/scdf/tasks/logs/mytask?platformName=default&schemaTarget=boot2This PR is going to exchange the protocol / host / port from the one of the reverse proxy so that it is going to look like this
http://reverse-proxy-url:443/scdf/tasks/logs/mytask?platformName=default&schemaTarget=boot2So
somedomain.deis replaced withreverse-proxy-url,8080is replaced with443andhttpwithhttpIf the server does not run behind a reverse proxy nothing is going to be changed - for the tests I wrapped the url into a try catch as there is no protocol / domain / port present.
Edit: fixes: #1994