fix(machines): Allow ports at the end of IPMI addresses LP#2087965 #5560
Conversation
Demo
Jenkins 
demos.haus 
| ); | ||
| // We use +2 here to include the `:` before the port number | ||
| const port = value.slice(closingBracketIndex + 2); | ||
| return isIPv6(ip) && !isNaN(parseInt(port)); |
There was a problem hiding this comment.
This usually works but insane users like myself might try: [aa:cf:f::34fF]: 4 5 6. Results in a parseInt(" 4 5 6") which is 4. I think it's still OK to merge for now but we might want to make this even tighter later.
There was a problem hiding this comment.
That's interesting! By the looks of it, parseInt ignores any leading whitespace, and ignores anything once it reaches the first whitespace after a number (only if the number is also the first non-whitespace text in the string).
I guess the simplest check for this would just be to check for any whitespace in the input, and treat the value as invalid if there is whitespace.
| } else if (value.split(":").length === 2) { | ||
| // This is an IPv4 address with a port number | ||
| const [ip, port] = value.split(":"); | ||
| return isIP(ip) && !isNaN(parseInt(port)); |
There was a problem hiding this comment.
as above. Could error out if the port number has anything but decimals.
| import { isValidPortNumber } from "."; | ||
|
|
||
| it("returns true for any number between 0 and 65535", () => { | ||
| for (let i = 0; i <= 65535; i++) { |
There was a problem hiding this comment.
That is a computationally expensive test for the function it tests.... Edge cases and some common ports might be enough. But I'm happy to merge if it runs quickly.
There was a problem hiding this comment.
Locally it seems to run quickly enough, but thought I'd get your thoughts first.
| * @param port The port number to check. | ||
| * @returns True if valid, false otherwise. | ||
| */ | ||
| export const isValidPortNumber = (port: number): boolean => { |
There was a problem hiding this comment.
I like that its port: number :)
Done
QA steps
[2008:d45::1]:8000Fixes
Fixes LP#2087965
Fixes MAASENG-4172