fix(32bit): drop darsyn/ip dependency #838
Conversation
Parse IPv6 addresses directly using built-in functions. Signed-off-by: Richard Steinmetz <[email protected]>
| $hex = bin2hex($addr->getBinary()); | ||
| $addr = inet_pton($ip); | ||
| if ($addr === false) { | ||
| throw new InvalidArgumentException('Invalid IPv6 address'); |
There was a problem hiding this comment.
@ChristophWurst Maybe you have a better idea on how to handle this a bit more gracefully?
There was a problem hiding this comment.
Doesn't look like inet_pton can fail on anything other than a truly invalid IP address value provided to it... Which seems highly unlikely given the source: either REMOTE_ADDR as provided directly by the web server (based on the functioning client IP connection) or from a proxy provided header (set also based on the actual connection there from the client).
So in the unlikely event it does fail, something is very very wrong (so a hard exception seems desirable + acceptable IMO).
There was a problem hiding this comment.
I think this approach is fine. \Darsyn\IP\Version\IPv6::factory would through too 👍
|
/backport to stable28 |
|
/backport to stable27 |
|
/backport to stable26 |
|
So... when will there be a new release of this app containing this fix? I'm still stuck at 27.1.5 because of this. Please continue to walk that last meter (or inches) of that mile. I think it would be possible to manually hack the changes of 967fa13 on ones already downloaded local app files, wouldn't it? As that would likely cause integrity warnings I'm really looking forward to see a new release Edit: I don't get the release and update mechanisms of this app.
How can I make sure I get this fix? |
|
Do not install the app from the app store. It is shipped starting from Nextcloud 25 and will be updated automatically every time you update your Nextcloud server. How to get this fix? -> Wait for the next Nextcloud patch or minor release and install it. Please refer to https://github.com/nextcloud/suspicious_login?tab=readme-ov-file#installation for more information. |
|
OK thanks that was pretty helpful. ...BUT it does not fully answer my question. 27.1.6 does not contain it (obviously, as me updating to that release initially discovered this dependency issue which lead to this fix in the end), BUT https://github.com/nextcloud/server/releases/tag/v27.1.7rc1 does not either. So let me specify my question: when will this fix arrive in which stable (series 27) release? I saw there's a https://github.com/nextcloud/suspicious_login/releases/tag/v27.1.7rc1... |
|
It should be included in 27.1.7 RC1 and will be included in the final 27.1.7 release. Testing an RC is always welcome but you are on your own risk. EDIT: The release of v27.1.7rc2 will also contain the fix, ofc. |
|
I can't see it in the https://github.com/nextcloud/server/releases/tag/v27.1.7rc1 release notes. Maybe I'm missing something. |
|
Also nothing in https://github.com/nextcloud/server/releases/tag/v27.1.7rc2 Now https://github.com/nextcloud/server/releases/tag/v27.1.7 is out. How can one be sure this fix is actually part of 27.1.7? |
|
The changelog is not extensive and it is included in the final release of 27.1.7. |
|
Confirmed working (27.1.7.2 update). |
Fix #837
Fix nextcloud/server#43157
Parse IPv6 addresses directly using built-in functions. I compared both binary representations of hundreds of random IPv6 addresses and they are exactly the same so we don't need to drop trained models.
Ref https://www.php.net/manual/en/function.inet-pton.php
Can be tested via: