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

Questionnements autour de l'utilisation de crypto-js dans le Semantic-data-provider #1158

Closed
mguihal opened this issue Sep 20, 2023 · 7 comments

Comments

@mguihal
Copy link
Contributor

mguihal commented Sep 20, 2023

Problématique

  • Actuellement Archipelago est buildé avec React-scripts 3 qui n'est pas compatible avec Node 18 dû à des corrections de sécurité. On ne peut le builder qu'avec Node 16 maximum, dont la version n'est désormais plus maintenue.
  • En mettant à jour vers React-scripts 5, la version de webpack passe de la v3 à la v5, et ne prend plus en charge nativement les polyfills des modules Node, dont le module crypto, utilisé notamment par le package semantic-data-provider

Composants concernés
frontend/semantic-data-provider

Proposition
Le souci se trouve dans l'utilisation du package crypto-js pour la génération d'un hash md5 ici https://github.com/assemblee-virtuelle/semapps/blob/master/src/frontend/packages/semantic-data-provider/src/dataProvider/utils/buildBlankNodesQuery.js#L23

Je ne maitrise pas du tout cette partie ni la complexité métier de ce fichier, mais je me questionne sur l'utilité d'avoir utilisé une telle lib pour l'usage qui en est fait. En effet, l'utilisation des hashs md5 ne semble utilisé que pour générer des noms de variables, mais sans réel but cryptographique.

Est-ce que ce serait pertinent de remplacer ce hash (et cette lib) par une autre système de génération aléatoire de chaines de caractères (par exemple un uuid), ou est-ce que les hash md5 ont réellement une utilité dans les requêtes Sparql ?

@srosset81
Copy link
Contributor

Merci pour la recherche
La bonne nouvelle c'est qu'on devrait pouvoir se passer de hash md5 avec la nouvelle méthode de détection automatique des blank nodes que je vais implémenter dans #1151

@srosset81
Copy link
Contributor

Bon pour finir, comme discuté dans le chat, on va garder quand même la possibilité d'indiquer les blank nodes à déréférencer, car ça peut améliorer les perfs, surtout si on a pas besoin de toutes les données. C'est juste que les blank nodes devrons être indiqué dans le code, sinon on utilisera la technique de détection automatique.
Du coup la fonction md5 sera toujours utilisée.
Tu te demandais pourquoi utiliser md5 ? Le but principal est de générer un hash qui est toujours le même.
Si tu vois une alternative qui serait compatible avec webpack 5, je suis preneur.
(Pour info, je vais passer sur ParcelJS pour builder les packages frontend. Ca simplifie beaucoup la configuration. Mais je sais pas si ça a un impact sur le problème que tu mentionnes).

@mguihal
Copy link
Contributor Author

mguihal commented Oct 10, 2023

Après de nouveaux tests, il semblerait que le passage à Parcel ait réglé le problème. La version 0.6.0-alpha.1 comporte encore le problème parce qu'une des dépendances n'avait pas été mise à jour (corrigé ici : #1175), mais une fois une prochaine version buildée, on devrait être bon 👍

@srosset81
Copy link
Contributor

Excellente nouvelle ! Je ferme cette issue du coup.

@rmkni
Copy link

rmkni commented Mar 8, 2024

Hi there! As a new contributor, I wanted to run the application (yarn start) and I faced this nodejs / openssl conflit

On master, the README proposes "yarn start" which seems to use react-scripts v3 not compatible with nodejs > 16.

If parceljs is used now, can we update the README to stop using yarn start?

If not, could we specify a nodejs version in package.json?

I can propose the changes in a "new joiner optimizations" PR, but I just wanted to check with you.

@srosset81
Copy link
Contributor

Hi @rmkni and welcome !

As a new contributor, I wanted to run the application (yarn start)

The "application"... you mean Archipelago ?
We are on SemApps repo here, so this creates some confusion. :)
If you are trying to run Archipelago, then I think that this PR from @mguihal assemblee-virtuelle/archipelago#162 is already trying to solve the problem you are talking about.

@rmkni
Copy link

rmkni commented Mar 8, 2024

Oups wow I missed that one (weirdly found nothing on archipelago so I moved here ^^') thanks that's exactly what I was looking for :D

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

No branches or pull requests

3 participants