HTTP + HTTPS is broken in sannzbd

[kavehv]

This finally works for me again and allows me to have both http and https running simulateously. There's some cleanup that could be done in that plugin as well (but I didn't have time to do it). I see a few hardcoded paths and references here and there.

[kavehv]

Also, in that script, the whole concept of a 'port' needs to be rehashed. As far as sab is concerned, you shouldn't really need to grab the https_port for anything other than checking the pid. Everything else should be based off of '^port'

[Influencer]

I'll take a look at it in the morning. I do need to cleanup the code, I too noticed the hardcoded paths, just lately haven't had much time to focus on it. On Oct 22, 2012 1:07 AM, "kavehv" notifications@github.com wrote:

This finally works for me again and allows me to have both http and https running simulateously. There's some cleanup that could be done in that plugin as well (but I didn't have time to do it). I see a few

hardcoded paths and references here and there.

You can merge this Pull Request by running:

git pull https://github.com/kavehv/UNplugged master

Or view, comment on, or merge it at:

https://github.com/Influencer/UNplugged/pull/5 Commit Summary

• Fix problems with https always running on same port

File Changes

• M sabnzbd_unplugged.plg (38)

Patch Links

• https://github.com/Influencer/UNplugged/pull/5.patch

• https://github.com/Influencer/UNplugged/pull/5.diff

  — Reply to this email directly or view it on GitHubhttps://github.com/Influencer/UNplugged/pull/5.

[kavehv]

I'll see if I can clean some of it up in my free time (which isn't plentiful, sad to say), but I just wanted to get this quick 'hotfix' in.

[Influencer]

May I ask what issue you were having with sab and the way it determined the port? During my testing it worked fine, sab creates the pid with the port being used (always the https port if its enabled), unless they changed that in recent releases.

I didn't have a chance today to test those changes, hopefully tomorrow. Was wondering about what the issue was in this.

On Tue, Oct 23, 2012 at 12:07 AM, kavehv notifications@github.com wrote:

I'll see if I can clean some of it up in my free time (which isn't plentiful, sad to say), but I just wanted to get this quick 'hotfix' in.

— Reply to this email directly or view it on GitHubhttps://github.com/Influencer/UNplugged/pull/5#issuecomment-9690089.

[kavehv]

Well, I typically set up sab to run in http on 8080 (default) and https on 9090 simultaneously. That way I can forward the https service through my router and not have to worry about passwords being transmitted in plaintext across the interwebs.

This always worked fine before, until I updated the PLG and sab version from 0.7.3 to 0.7.4 yesterday. When I did it with this version, it kept setting both ports to 9090 or 8080, thus http and https would try to run on the same port and result in a trainwreck. I'd have to stop sab, turn off https manually, and restart it to get it going. A little digging led me to some lines in the PLG that seem like they are trying to do more than they really need to. The simple (and more predictable) the implementation, the better, imho.

[Influencer]

So this is a change in Sab and not so much the plug-in? How long had it been since you had updated the plug-in previously? The way I determine which port to look for has been in the plug-in for quite some time since commit 7a2794aadd on July 7th.

[kavehv]

I think that's the last time I updated.

Sent from my mobile

On Oct 22, 2012, at 9:27 PM, Influencer notifications@github.com wrote:

So this is a change in Sab and not so much the plug-in? How long had it been since you had updated the plug-in previously? The way I determine which port to look for has been in the plug-in for quite some time since commit 7a2794a on July 7th.

— Reply to this email directly or view it on GitHub.

[Influencer]

Ok, I'm seeing the issue, when you enable https as the default 9090, the plug-in is setting the regular port to 9090 as well.

The reason your fix is working is when https is enabled sab creates the pid file with the https port. It actually doesn't address the issue, which is the plug-in is changing the regular port to whatever is set on the plug-in ui(by default when https is enabled it will show the https port, you can change this but it will appear as if its not "setting" and will stil appear to be the https port).

I'm not going to pull this right now as the bigger bug needs to be addressed. I'll leave it open for now until I can sit down and test a few things.

I'm not even 100% sure how the edits you've made fixed your issue, the main part you changed was for the counter when starting the app, if you were to set the port from the plug-in ui again, it would create the same issue(only if you tried to start without changing the "PORT" to something other than what https runs on(default 9090).

The issue is line 249 and how it handles updating the port. I need to check whether https is enabled and update that port instead of the current behaviour where it always updates the regular port without regard to which port its reporting.

[kavehv]

Yeah, I agree with your assessment. If I get more free time in the coming weekend, I may be able to get to fixing that bigger bug, but for the moment this is working alright.

Sent from my mobile

On Oct 22, 2012, at 10:14 PM, Influencer notifications@github.com wrote:

Ok, I'm seeing the issue, when you enable https as the default 9090, the plug-in is setting the regular port to 9090 as well.

The reason your fix is working is when https is enabled sab creates the pid file with the https port. It actually doesn't address the issue, which is the plug-in is changing the regular port to whatever is set on the plug-in ui(by default when https is enabled it will show the https port, you can change this but it will appear as if its not "setting" and will stil appear to be the https port).

I'm not going to pull this right now as the bigger bug needs to be addressed. I'll leave it open for now until I can sit down and test a few things.

I'm not even 100% sure how the edits you've made fixed your issue, the main part you changed was for the counter when starting the app, if you were to set the port from the plug-in ui again, it would create the same issue(only if you tried to start without changing the "PORT" to something other than what https runs on(default 9090).

The issue is line 249 and how it handles updating the port. I need to check whether https is enabled and update that port instead of the current behaviour where it always updates the regular port without regard to which port its reporting.

— Reply to this email directly or view it on GitHub.

[overbyrn]

Although not exactly the same issue as kavehv has mentioned above, the logic for whether https is enabled and if so what port is used does not take account of when https port is left empty. SAB allows the user to have an empty https port field in its webgui, whilst https itself is enabled. This causes SAB to use the value in the regular port field instead for access over https. This presents problems for the plugin as a check is first done to see if https is enabled and if so the https port value is assumed to be in https_port. A further check needs to account for whether http_port is empty and fallback to non-https port field instead. To reproduce; blank out the https port field in sab, enable https and maintain a value in regular port. Visit plugin webgui page; port will be empty and as a result present issues when calling rc start function as port param value is passed empty. Workaround is to implicitly have a value in https port, however the downside is that this is one more listening port which is not strictly needed.

[Influencer]

I'll revisit this, I've gone at it a couple times but couldn't get it to act how I want.

Overbyrn, you mentioned in another comment that you had another PR that dealt with HTTPS, does it address this issue?

[overbyrn]

I hope so yes. Will do a PR so you can walk through the code

[Influencer]

Alright, I'll close this one now...if something else isn't addressed you can re-open this PR