Validate and clean sysobjectid in NetboxTypeForm

[johannaengland]

Closes #2566.

[codecov]

Codecov Report

Merging #2584 (c8bb100) into 5.6.x (b7190b9) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head c8bb100 differs from pull request most recent head e82ff41. Consider uploading reports for the commit e82ff41 to get more accurate results

@@            Coverage Diff             @@
##            5.6.x    #2584      +/-   ##
==========================================
- Coverage   53.89%   53.86%   -0.04%     
==========================================
  Files         558      558              
  Lines       40612    40594      -18     
==========================================
- Hits        21887    21864      -23     
- Misses      18725    18730       +5     

Impacted Files	Coverage Δ
python/nav/web/seeddb/forms/__init__.py	59.14% <100.00%> (+1.82%)	⬆️

... and 10 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[github-actions]

Test results

     12 files       12 suites   12m 7s ⏱️
3 223 tests 3 127 ✔️   96 💤 0 ❌
9 144 runs  8 856 ✔️ 288 💤 0 ❌

Results for commit e82ff41.

♻️ This comment has been updated with latest results.

[lunkwill42]

It looks like you've actually latched on to the third of three complementary solutions suggested in #2566 - but this doesn't seem to do what is actually suggested: Prevent someone from entering a sysObjectID value with a leading . (in fact, you've added to test to ensure a leading . is explicitly allowed).

An alternate twist to this might be to just strip the leading . before storing the value.

On another note, you might want to have a look into the class nav.oids.OID for this. This could be used to validate an OID - but this class will allow leading .-s (since these are normally allowed, it's just type.sysobjectid that's weird).

[johannaengland]

Yes, that is exactly what I've done: instead of not allowing the user to enter a sysObjectID value with a leading . I simply strip that leading period before storing it.

And I had a look at the OID class before, but it doesn't provide any validation, so I used that as inspiration to validate sysObjectID in the form.

[lunkwill42]

Yes, that is exactly what I've done: instead of not allowing the user to enter a sysObjectID value with a leading . I simply strip that leading period before storing it.

Indeed, I managed to misread that part :)

And I had a look at the OID class before, but it doesn't provide any validation,

I beg to differ:

>>> from nav.oids import OID
>>> OID('foobar')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/mvold/d/nav/python/nav/oids.py", line 53, in __new__
    return tuple.__new__(cls, oid)
ValueError: invalid literal for int() with base 10: 'foobar'
>>> OID('1,2,3,4')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/mvold/d/nav/python/nav/oids.py", line 53, in __new__
    return tuple.__new__(cls, oid)
ValueError: invalid literal for int() with base 10: '1,2,3,4'
>>> OID('1.2.3.4')
OID('.1.2.3.4')
>>> OID('.1.2.3.4')
OID('.1.2.3.4')
>>> 

What good would the OID class be if it didn't validate its input?

[lunkwill42]

Nice - just nitpicking the changelog again ;)

[hmpf]

Good 'nough

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information