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

cleanup http handling logic #295

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

cleanup http handling logic #295

wants to merge 1 commit into from

Conversation

mwarning
Copy link
Member

@mwarning mwarning commented Sep 6, 2018

This is a first step to make the code clearer.

@mwarning
Copy link
Member Author

mwarning commented Sep 6, 2018

@bluewavenet maybe you can give it a test.

@bluewavenet
Copy link
Contributor

@mwarning
Sorry for the delay!
It compiles and runs fine at least on a quick test on a fas test system.
I will test all the combinations fas / binauth / basic mode tomorrow, but looks good.

@bluewavenet
Copy link
Contributor

@mwarning
Works perfectly with all combinations of basic/binauth/fas :-D

@mwarning
Copy link
Member Author

good to know - thanks!

@mwarning
Copy link
Member Author

@bluewavenet is status page also displayed? Do you have some css or image file included?

@bluewavenet
Copy link
Contributor

@mwarning
Yes, status with splash image, css (with custom tags :-p ). Works perfectly.
I rebased from master and re-compiled just to make sure.

@bluewavenet
Copy link
Contributor

Wait - might be an issue..... couple more tests....

@bluewavenet
Copy link
Contributor

bluewavenet commented Sep 12, 2018

@mwarning
Actually no it is not working with status.html in basic and/or binauth mode, but does with fas enabled.
On clicking "back" on the browser then clicking "Continue" again, NDS returns:
http://10.168.1.1:2050/nodogsplash_auth/?tok=acfa9be5&redir=http%3A%2F%....... etc

Should be:
http://10.168.1.1:2050/status.html?tok=acfa9be5&redir=http%3A%2F%.... etc

@bluewavenet
Copy link
Contributor

@mwarning
I mistyped my last comment so edited it.

@mwarning
Copy link
Member Author

ok, looks like I have some fixing to do.

@lynxis
Copy link
Member

lynxis commented Jul 8, 2020

@mwarning do you like to redo this PR again onto the current master? It would be great.

@mwarning
Copy link
Member Author

mwarning commented Jul 8, 2020

I will do it this week.

@mwarning
Copy link
Member Author

rebase is done. But I need to revisit/test the logic to check if this does not break anything.

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.

None yet

3 participants