tladesignz / IPtProxy

Obfs4proxy and Snowflake Pluggable Transports for iOS
MIT License
53 stars 12 forks source link

StateLocation variable is never initialized #49

Closed stevenmcdonald closed 1 year ago

stevenmcdonald commented 1 year ago

so fixEnv() panic()s (at least in my fork, but it looks like the IPtProxy code is the same)

As far as I know, the only thing the TOR_PT_STATE_DIRECTORY is used for is Lyrebird's logging, but the spec says it should persist across runs

I'm using os.MkdirTemp() with a * to create a secure temp dir in a test build of my fork, but I don't like it because it won't persist across calls. I'm not really a mobile developer (despite maintaining a fork of this :) ), so I'm not sure what the right thing to do is, especially that will work on both Android and iOS

I'm happy to submit a patch, but I want to get it right for future PTs that might expect that dir to be stateful

tladesignz commented 1 year ago

Looking at the code, it seems pretty clear, why it panics:

https://github.com/tladesignz/IPtProxy/blob/master/IPtProxy.go/IPtProxy.go#L388-L424

If you look at the panic message, it should tell you the exact reason.

Until you can show a configuration which produces a panic which it arguably shouldn't, I won't accept any patches.

stevenmcdonald commented 1 year ago

I'm not trying to push any code on you, I'm just saying I'm willing to help if it needs a fix, rather than expecting someone else to fix it for me :)

Looking back at my report, I'm slightly embarrassed, I should have provided more info, sorry about that. The panic was showing that StateLocation is set to "", and I looked though the code and couldn't see where or how it was set.

We didn't realize there was a IPtProxy.setStateLocation() available to the calling code (I guess gomobile provides that?), which is what we were missing.

Thanks for the prompt response, and sorry to waste your time.

tladesignz commented 1 year ago

I'm not trying to push any code on you, I'm just saying I'm willing to help if it needs a fix, rather than expecting someone else to fix it for me :)

All good.

We didn't realize there was a IPtProxy.setStateLocation() available to the calling code (I guess gomobile provides that?), which is what we were missing.

It's provided in IPtProxy.go. It was always there, but recently I had to remove the default, because on Android, I had to realize, there's just no way to provide a safe default.

That's why it's suddenly crashing. Sorry.

It's documented, but of course nobody re-reads documentation for a seamingly simple lib.

Thanks for the prompt response, and sorry to waste your time.

No worries.