Breaking LinkValidator (Serialised Array vs. JSON)


(Mathias Schreiber) #1

Discussion Topic

Sybille Peters is willing to take over LinkValidator.
The intention is to make the overall module easier to use, revise parts of the architecture (###MARKERS_YOU_KNOW###) and have it provide more value overall.

While digging through the code we found that it uses a serialised PHP array in its data storage.
We’d like to replace this with a JSON data structure.
Security is is most obvious benefit, being able to query and or update records directly is another one.

Impact

  • Manually inserting serialised arrays will no longer work.
  • Hooks for LinkValidator that insert data are out of luck.

Possible Migrations

  • Add a new field for url_response (like url_response_json). Upon data retrieval we could check if unserialize of url_response returns an array and if so, transfer the data over into the JSON field.

Pro

  • One place less that uses unserialize
  • One attack vector less

Con

  • Arguably breaking chance.

Remarks and notes

Organizational

Topic Initiator: Mathias Schreiber
Topic Mentor: Christian Kuhn


(Kay Strobach) #2

Go for it, please introduce a new hook / signal or fluid config to adjust output.


(Philipp Gampe) #3

As one of the original maintainers, go ahead. This module could really benefit from some love.


(Stefan Neufeind) #4

+1 You had my vote the second you wrote “one less unserialize” :slight_smile:


(Stefan Busemann) #5

+1 for the change - i find only two extensions in TER, which are somehow related to the validator.


(Alexander Opitz) #6

In the last days, I wrote the ppi_tvplus_linkvalidator plugin, so TV flexform can be searched. So I looked inside the code and yes, it is a little hell. JSON or serialized, I don’t mind, but I would like to have a better possibility as extension to add functionality to Link Validator. So I offer to help reviewing the patches.
And, I’m fine with breaking, as I didn’t found another TER extension which depends on LinkValidator.


(Mathias Schreiber) #7

Searching for links in Flexforms is something I didn’t think of yet, but I think it should be available.
Although I consider content (other than configuration) a no-go in flexforms people seem to actively use it.
I woud suggest getting together with Sybille once she has the basics of link-analysis down so flexform lookup of links becomes a first-class citizen of linkvalidator.


(Sybille Peters) #8

@opi99 About extensibility: the currently available hooks should definitely be preserved and possibly be extended. I think it might be a good idea, if you also comment the already existing issue (https://forge.typo3.org/issues/59502), I will be watching it closely.

The problem with linkvalidator is: It looks easy and straightforward at first, but it’s actually not. There are lots of things to consider and these are easily overlooked.

Having more people to review the patches and test would definitely be a huge help.


(Riccardo De Contardi) #9

positive vote from me. Will gladly help with tests.


(Jigal Van Hemert) #10

If voting is active I’ll give it a +1 (long answer to get past the 20 char lower limit of a reply)


(system) #11

This topic was automatically closed after 14 days. New replies are no longer allowed.