Prevent infinite loops on validation

Attached patch.

The last submitted patch, 2: redirect-3380747-detect-loops-on-validation-2.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Even if tests fail I'd like to get maintainers feedback before working more on this.

I think it would be really good to avoid having to save a redirect that may be invalid. While I like not having to change the underlying code, is there a way we could change the recursive function to key off of the hash instead of the redirect ID?

I don't think this is gonna work.

For finding not-direct loops we need

     $rid = $this->connection->query('SELECT rid FROM {redirect} WHERE hash IN (:hashes[]) ORDER BY LENGTH(redirect_source__query) DESC', [':hashes[]' => $hashes])->fetchField();

to return something.

The last submitted patch, 7: redirect-3380747-detect-loops-on-validation-7.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Of course #2 failed, as this approach will only be valid for new redirects. For existing ones, on edit form, our temp redirect conflicts with the original one because of the hash unique key constraint.

So we can only prevent this from happening in new redirects, not in edits.

One thing we could do better here in #9 is checking if the entity is not new, but the new hash is not equals to the original hash (which means the path changed). Then we could check that in edits too.

ankitdebnath → closed merge request !58

@ankitdebnath can you be more clear about that? What was fixed? Yes, the detection works on runtime, this issue is about validating it before it happens.

I've created a new MR and fixed an issue where if you forgot the leading slash, a URL \InvalidArgumentException bubbled up and crashed the page.

Maybe this should be postponed on 📌 Public method Redirect::generateHash() should unify path before generating the hash Active

Anybody → changed the visibility of the branch 3380747-validate-redirects-on-save to hidden.

This should indeed have tests, would be great if they could be added to the MR.