Improve type safety, unit test coverage, and usage of crypto routines.

Review Request #1659 — Created May 15, 2019 and updated

guest7537
Review Board
3880a2c...
demo
guest7011, guest7015
This adds and improves the string type checks for the crypto and SSH
functions, ensuring we catch invalid input before hitting a cryptic
error message. It also fixes a couple of compatibility issues with
Python 3, involving division and an improper decode on a Unicode string.

The bulk of the change improves test coverage of these functions,
ensuring that all the types match what we expect. As part of this, the
method used to generate SSH keys for the tests have changed to create
stable keys for testing (so we can ensure that fingerprints don't change
across test runs) and to do so in a way that ensures we only load the
keys on first use, reusing them for any future tests.

Testing Done:
Unit tests pass on Python 2.7/Django 1.6 and Python 3.7/Django 1.11
(with other in-progress changes).

Reviewed at https://reviews.reviewboard.org/r/10509/

The code is perfect.

Description From Last Updated

perfect function improvements.

guest6388guest6388

Another one.

guest2715guest2715

Testing inline review.

guest2715guest2715

what the heck is this

guest166guest166

this is the comment

guest6622guest6622

This looks like a silly thing to do.

guest3318guest3318

Since you have tests can you add the test run output or pipeline link to the testing done section?

guest7537guest7537

Please elaborate the comments

guest6785guest6785

Opening an issue

guest4527guest4527

But I don't like this. But if you don't care about my opinion, just close it.

guest7537guest7537

bakbass code hai ye to

guest4804guest4804

Wait, this looks like my key!

guest7537guest7537

public too bad :(

guest4804guest4804

What the hack is this?

guest4804guest4804
guest7537
guest7537
Review request changed

Testing Done:

  +

The code is perfect.

guest7537
  1. 
      
    1. I'll have to get back to your review comments tomorrow.

  2. Since you have tests can you add the test run output or pipeline link to the testing done section?

  3. reviewboard/ssh/tests.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Wait, this looks like my key!

    1. I alwayse just use 12345

  4. reviewboard/ssh/utils.py (Diff revision 1)
     
     

    TIL you can detect the py version!

  5. 
      
guest6877
  1. Ship It!
  2. 
      
guest6877
  1. please

  2. 
      
guest4476
  1. Ship It!
  2. 
      
guest7537
  1. You can ship this without addresssing the issue.

  2. But I don't like this. But if you don't care about my opinion, just close it.

  3. 
      
guest775
  1. xxx

  2. 
      
guest541
  1. Ship It!
  2. 
      
guest2715
  1. 
      
  2. reviewboard/scmtools/crypto_utils.py (Diff revision 1)
     
     
     
     
     

    Another one.

  3. reviewboard/scmtools/crypto_utils.py (Diff revision 1)
     
     
     
     
     

    Testing inline review.

  4. 
      
guest4804
  1. 
      
  2. bakbass code hai ye to

  3. reviewboard/ssh/tests.py (Diff revision 1)
     
     

    public too bad :(

  4. reviewboard/ssh/tests.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     

    What the hack is this?

  5. 
      
guest4804
  1. Ship It!
  2. 
      
guest4527
  1. 
      
  2. Opening an issue

  3. 
      
guest6388
  1. tesst review function as demo user.

  2. perfect function improvements.

  3. success

guest166
  1. 
      
  2. reviewboard/scmtools/crypto_utils.py (Diff revision 1)
     
     

    what the heck is this

  3. reviewboard/ssh/utils.py (Diff revision 1)
     
     

    explain pls

    1. I don't know how to compuuutoerueors hlapel

  4. 
      
guest6622
  1. 
      
  2. reviewboard/scmtools/crypto_utils.py (Diff revision 1)
     
     

    this is the comment

  3. 
      
guest6785
  1. 
      
  2. Please elaborate the comments

  3. 
      
guest6785
  1. Ship It!
  2. 
      
guest3318
  1. 
      
  2. reviewboard/scmtools/crypto_utils.py (Diff revision 1)
     
     

    This looks like a silly thing to do.

  3. 
      
guest3318
  1. Ship It!
  2. 
      
guest6100
  1. Ship It!
  2. 
      
Loading...