typo3-realurl icon indicating copy to clipboard operation
typo3-realurl copied to clipboard

[BUGFIX] Consistently integrate addWhereClause

Open tantegerda1 opened this issue 6 years ago • 6 comments

The addWhereClause is twice prefixed by a space and twice not. This change makes all occurences consistently include a prefix space, thus avoiding possibly invalid sql.

tantegerda1 avatar Sep 27 '17 10:09 tantegerda1

Other occurences, already prefixed with a space: https://github.com/dmitryd/typo3-realurl/blob/58a17ac0b561729412b2afd53b6a106c3755192e/Classes/Decoder/UrlDecoder.php#L318-L321 https://github.com/dmitryd/typo3-realurl/blob/58a17ac0b561729412b2afd53b6a106c3755192e/Classes/Encoder/UrlEncoder.php#L420-L422

On the other hand, the wiki already states:

addWhereClause is an additional WHERE clause that must start with ' AND'.

(https://github.com/dmitryd/typo3-realurl/wiki/Configuration-reference#lookuptable) so another option would be to consistently not include the space before the additional where-clause.

The current code poses a little trap, because it is not immediately obvious that you have a wrong configuration, since it works in some cases.

tantegerda1 avatar Sep 27 '17 11:09 tantegerda1

I'm happy to change the pull request into removing the additional space on every occurence if you like that better. I just recommend to avoid the inconsistency.

tantegerda1 avatar Sep 27 '17 11:09 tantegerda1

Read the docs:

addWhereClause is an additional WHERE clause that must start with ' AND'. For example: ' AND deleted=0 AND hidden=0'.

dmitryd avatar Oct 05 '17 07:10 dmitryd

Basically the space should be in addWhereClause. The space in the code is probably left after rearranging or removing other conditions. It should be removed.

dmitryd avatar Oct 05 '17 07:10 dmitryd

Changed the pull request to consistently not include a space.

tantegerda1 avatar Oct 05 '17 09:10 tantegerda1

I like the change but I am afraid that there will be people who do not use space and will complain that their setup suddenly stopped working.

dmitryd avatar Apr 02 '18 11:04 dmitryd