-
-
reviewboard/scmtools/perforce.py (Diff revision 1) No need for this. You'll get it for free from os.
-
-
reviewboard/scmtools/perforce.py (Diff revision 1) Not really a big deal, but I noticed this block is repeated twice (except for the '-c' and '-p', certfile). Is it worth consolidating some of this? A run_stunnel with provided args?
-
reviewboard/scmtools/perforce.py (Diff revision 1) We should probably bullet-proof this a bit in case the contents of stunnel.pid can't be converted into an int. Probably just catch ValueError and bail somehow?
-
reviewboard/scmtools/perforce.py (Diff revision 1) I don't see found ever becoming True. Just while 1? Do we want to risk this somehow infinite looping? Maybe try a max number of times?
-
-
-
reviewboard/scmtools/perforce.py (Diff revision 1) We do this in multiple places. Can we consolidate?
-
-
Add a stunnel mode for Perforce.
Review Request #2482 — Created July 15, 2011 and submitted
Add a stunnel mode for Perforce. Out of the box, Perforce doesn't support any sort of secure connectivity. This is an artifact of the sorts of environment that Perforce is usually run in, but isn't really a modern approach to life (people want security even inside the firewall). The suggested workaround for this is to run a stunnel proxy on the server, and a stunnel client proxy on the client. Users can then connect using a localhost: P4PORT and the encryption will be handled by stunnel. This change adds support for this configuration, by adding a special stunnel: prefix to the P4PORT setting. When someone uses this prefix, the port will be interpreted as a stunnel server to connect to. We then spin up a stunnel client on a random port and use that as the P4PORT. When we're done, we disconnect perforce and kill the stunnel process. This is a little complicated in implementation because stunnel can either run in the foreground (in which case we race to see whether we try to connect before it listens on the local port), or it can run in the background (in which case we race the -filesystem- for when it writes out its PID file). I've chosen the latter case as the lesser of the two races.
- Unit tests - Tested setting up a stunnel server and adding that repository via stunnel:localhost:port, and posted a review request to it. The changeset contents and diff loaded just fine.
| Description | From | Last Updated |
|---|---|---|
|
No need for this. You'll get it for free from os. |
 chipx86
|
|
|
Can you make this inherit from object? |
chipx86
|
|
|
We should probably bullet-proof this a bit in case the contents of stunnel.pid can't be converted into an int. Probably … |
chipx86
|
|
|
I don't see found ever becoming True. Just while 1? Do we want to risk this somehow infinite looping? Maybe … |
chipx86
|
|
|
Should inherit from object. |
chipx86
|
|
|
Too many blank lines. |
chipx86
|
|
|
The blank line isn't really needed. |
chipx86
|
|
|
Shouldn't we pass use_stunnel? |
chipx86
|
|
|
To specify a password in the p4 command line, you should use the upper case -P option, not the lower … |
FA Faller.Gyula |
Change Summary:
Syntax fix, testing done.
Testing Done: |
|
||||||
|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+354 -93) |
FA
-
Found a serious typo.
-
reviewboard/scmtools/perforce.py (Diff revision 3) To specify a password in the p4 command line, you should use the upper case -P option, not the lower case -p, which means the port instead. (See the line 131 in the original file.) This is a serious regression bug since make it impossible to use ReviewBoard with Perforce if you need to supply a password. I suggest to correct it in the final 1.6.