pwned-passwords-django icon indicating copy to clipboard operation
pwned-passwords-django copied to clipboard

ValueError in _get_pwned() when /range returns hash that exists more than 999 times

Open linusg opened this issue 2 years ago • 4 comments

image

To repro view the result of https://api.pwnedpasswords.com/range/D95BE, which contains D2DC6572B8419EFAFA03DEEF501F1AA32C2:1,352 and FAFB8A7D40893D2FB8F67FACB0E982DBC3B:1,034 as of now.

linusg avatar Aug 25 '22 10:08 linusg

Snap! I just reported that as well. Interesting that it's just started. I assume new breaches have been loaded in

StevenMapes avatar Aug 25 '22 10:08 StevenMapes

Update: even though the linked URL now reports results without commas, I'm still seeing this in the wild on other URLs.


Hm, I wonder if the HIBP API itself briefly started introducing commas in the serialized response (either by mistake or on purpose) and then returned to the original comma-less output?

I've seen this issue myself, but can't seem to reproduce now.

The example link, https://api.pwnedpasswords.com/range/D95BE, now includes these lines (note the absence of commas):

D2DC6572B8419EFAFA03DEEF501F1AA32C2:1352
FAFB8A7D40893D2FB8F67FACB0E982DBC3B:1034

DavidCain avatar Aug 27 '22 17:08 DavidCain

Update: even though the linked URL now reports results without commas, I'm still seeing this in the wild on other URLs.

Hm, I wonder if the HIBP API itself briefly started introducing commas in the serialized response (either by mistake or on purpose) and then returned to the original comma-less output?

I've seen this issue myself, but can't seem to reproduce now.

The example link, https://api.pwnedpasswords.com/range/D95BE, now includes these lines (note the absence of commas):

D2DC6572B8419EFAFA03DEEF501F1AA32C2:1352
FAFB8A7D40893D2FB8F67FACB0E982DBC3B:1034

I last had it happen to a client 24 hours ago but switched them over to the fork I created to submit the PR. I think we should all assume it's still doing it and will in the future so should continue to strip out and formatting that may be there. Really that's both commas and periods depending on the country

StevenMapes avatar Aug 27 '22 20:08 StevenMapes

I think we should all assume it's still doing it and will in the future so should continue to strip out and formatting that may be there.

Oh, I totally agree - it definitely seems safe to strip , and . if found. Since the value is an integer count, there's no need to consider , or . denoting a fractional value/float.

I'm still surprised that the folks at HIBP would be including human-readable separators (in a specific localization -- US, maybe?), in the output of a machine-readable API. This still feels like an error on their end.

DavidCain avatar Aug 27 '22 20:08 DavidCain

I'm terribly sorry that this took so long -- for some reason GitHub never emailed me about the newly-opened issue, and otherwise I don't really check on packages that are (usually) this stable. I've merged the PR and pushed version 1.6.1 to PyPI, which should fix this.

In the future, please don't hesitate to bother me loudly via email or any other contact method when a bug of this severity seems to be going unattended.

ubernostrum avatar Dec 26 '22 08:12 ubernostrum