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

Avoid jQuery and use fetch API for Ajax #3

Open
gagarine opened this issue Apr 8, 2017 · 10 comments
Open

Avoid jQuery and use fetch API for Ajax #3

gagarine opened this issue Apr 8, 2017 · 10 comments

Comments

@gagarine
Copy link
Contributor

gagarine commented Apr 8, 2017

jQuery is pretty heavy library. I saw we are using it for a couple of DOM manipulation and for Ajax request.

For DOM manipulation it's very easy to use native JS.

For ajax request it's a bit more complicated but we can use the fetch API. It's now supported on Firefox, Chrome and even edge: https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API

@tomlutzenberger
Copy link
Member

I totally agree. I want to get rid of it.
But keep in mind, that some crypt jquery plugin is used. Maybe we need an alternative for that.

How about fetch API in Opera and Vivaldi? Maybe we don't need much effort to make it work in those as well.

@gagarine
Copy link
Contributor Author

gagarine commented Apr 8, 2017

Opera is supported to http://caniuse.com/#search=fetch . I guess Vivaldi to because it's based on blink, didn't find the information.

About the crypto, we are only using md5 right? I think we can use this one: https://github.com/blueimp/JavaScript-MD5

@tomlutzenberger
Copy link
Member

Great, thanks for the research.

Those are not the same. Have a look at src/lib/. There's a jquery.crypt.js and a md5.js. The latter one is a standalone script, so I think we can keep it.
I still haven't figured out, why the crypt stuff is needed, though.

@shanness
Copy link
Collaborator

shanness commented Apr 9, 2017

The crypt stuff is needed for privacy. We don't send URL's to the server, we send hash's instead. This is to protect users privacy. i.e. we can only know what URL they are viewing if it's already in the database.

@tomlutzenberger
Copy link
Member

Oh okay, that makes sense of course. But I'm not sure if MD5 is the best choice.
It's not really secure. We should look at it some time.

@shanness
Copy link
Collaborator

shanness commented Apr 9, 2017

I just did some reading on it, and it doesn't seem relevant to our use (privacy instead of security). Seems fine for us. If you really think it's an issue, could you provide a link and explanation of why.

@tomlutzenberger
Copy link
Member

Maybe I thougt too far. We'll leave it then.

@gagarine
Copy link
Contributor Author

gagarine commented Apr 9, 2017

md5 should be avoided in all cases. It's pretty easy to decode it and it have collision problems. I think md5 should only be used for data integrity check. SHA-2 is a better option in my opinion. Check https://en.wikipedia.org/wiki/MD5

Say that, it's perhaps not the top priority. But if changing that on the server side is not a big deal, that will be a plus.

@shanness
Copy link
Collaborator

shanness commented Apr 9, 2017

I read that link. Agreed it's not perfect, but I don't think collisions would be an issue on such short stuff as URL's. I had a look around, and it seems all the online decoders use an existing databases of hashes. Show me a link to how to decode it without prehashing stuff and I'd consider changing it (from my reading it's just brute force). The problem with changing it would be that users of the old plugin would break. Any hash algorithm would have the same pre-hashed database issue.

@gagarine
Copy link
Contributor Author

gagarine commented Apr 14, 2017

This is not enhancement. This is a critical issue: our jQuery version is not supported anymore (and I do have warning in Firefox) and updating is not possible (API break).

I removed some of the jQuery call, but their is still a lot of works.

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

No branches or pull requests

3 participants