Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password must not compress too well using gzip #11

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

mithodin
Copy link

No description provided.

@olikami
Copy link

olikami commented Mar 10, 2022

This would require to change the type of the worker to "webpack". Currently, it gives the following error:

Error: Something went wrong with the request to Cloudflare...
Uncaught SyntaxError: Cannot use import statement outside a module
  at line 1
 [API code: 10021]

@mithodin
Copy link
Author

Ah, do I need to use require() instead?

@mithodin mithodin force-pushed the feat/not-well-compressable branch from 424ef9f to 882f2f6 Compare March 11, 2022 08:08
@mithodin
Copy link
Author

@troyhunt I see quite a few other PRs being merged since I opened this one, if you're not interested in this please lmk so I can stop keeping it mergeable.

@troyhunt
Copy link
Owner

Hey @mithodin, sorry for not responding earlier. The main reason I've been hesitating on this one is that as more PRs have come in and more crazy conditions added, I've been thinking: what are the best criteria to infuriate, but also waste as much of the spammer's time as possible. To do this, we need to keep them engaged and if the criteria is too foreign to them (such as the compressability of data), that could kill their interest and prematurely end the time-wasting exercise. That said, I might have to revisit some of the existing contributions as well.

I'm going to leave this one here for the moment, I just wanted to make sure I gave you a good explanation. Hope that makes sense.

@mithodin
Copy link
Author

👍 thanks, makes sense. I'm not sure if a different wording for the message, like "Password entropy too low" or "Password too repetitive" would make it better? Could still make someone just quit, I guess.

@MichaelNMaggs
Copy link

MichaelNMaggs commented Mar 25, 2022

Not a good one, I think. The vast majority of people will think "what on earth does that mean?" and just give up

@@ -1,7 +1,25 @@
const { gzipSizeSync } = require('gzip-size');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need importing package which actually compresses data with zlib?

Comment on lines +9 to +14
const baseline = gzipSizeSync('a');

function compressesTooWell(password) {
const size = gzipSizeSync(password);
return (size - baseline) < (password.length - maxAllowedCompression);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will be a waste of computing resources. Instead we could just put RLE here for that


return {
passwordIsInvalid: compressesTooWell,
message: `Password must not be too repetitive`,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be phased as Password must not have consecutive characters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants