search

opentechinstitute / luci-commotion

Commotion configuration pages for the LuCI web interface
GNU General Public License v3.0
11 stars 17 forks source link

LuCI status pages trigger infinite redirect #440

Closed gradyoti closed 9 years ago

gradyoti commented 9 years ago

After running setup wizard, admin and status pages fail to load. Does not appear to be a networking issue.

seamustuohy commented 9 years ago

By infinite redirect do you mean a looping set of redirects or that it stalls?

jheretic commented 9 years ago

He means that the browser is giving the error "The page isn't redirecting properly. Firefox has detected that the server is redirecting the request for this address in a way that will never complete."

gradyoti commented 9 years ago

Indeed.

seamustuohy commented 9 years ago

Ahhh, might be an error in how the https redirect in the header is parsing and then redirecting the current page. It should just change the http to https, but it might be cutting out the wrong part.

jheretic commented 9 years ago

Now that I've gotten a chance to look at it, it looks like you were right @elationfoundation. The crypto.check_https function loops forever, though it isn't because it cuts out the wrong part of the string. It looks like the environment no longer passes an HTTPS element, which is how the function previously determined that the redirect completed. I can change it to a env.SERVER_PORT ~= 443 check to 'fix' it, but that's kind of crappy. It's probably fine for our setup, but I don't like that it doesn't actually directly signal whether the handshake completed or not (if we were running HTTP on port 443 it would still complete). Not a big deal, but anyone have any better ideas about how to do that check?

dismantl commented 9 years ago

Looking at the source of uhttpd2, it looks like it could easily be patched to provide that env var. How would you feel about that? It would be a good opportunity to upstream it as well.

jheretic commented 9 years ago

I think we can submit an upstream patch, but I'm just going to make the SERVER_PORT change for now. It works for our setup, and it won't require that we patch or otherwise host our own uhttpd2 package. However, we can still submit it as an upstream issue, as I think it's a worthwhile addition, and switch back if it gets integrated.

dismantl commented 9 years ago

fixed by https://github.com/opentechinstitute/luci-commotion/pull/448.

dismantl commented 9 years ago

uhttpd2 patch was submitted upstream. For reference: https://github.com/dismantl/uhttpd2/commit/ac934288e9b68c1bf07136b420fe9581a9177046