Function preg_replace could return null in case of an error

[xmorave2]

No description provided.

[demiankatz]

Thanks for this! I'll wait to see what @EreMaijala thinks before merging. In the meantime, a couple of small things to consider:

1.) Do you mind adding a bullet point about this fix to the "Fixed" section of the "next release" section of CHANGELOG.md? (something like "Do not allow the MarcXml serialization's toString method to illegally return null." would work).

2.) Is there an easy way to cover this case in the test suite? If not, it's probably not a major problem, but if you had a specific test case that led you to make this adjustment, it's probably a good idea to incorporate it to prevent regressions.

Thanks again!

[EreMaijala]

@xmorave2 Is there a real world case where this happens? I wonder if it would make sense to return the unclean XML instead of an empty string?

[demiankatz]

...or, alternatively, just throw an exception if this is not really an expected situation.

[xmorave2]

@EreMaijala Yes, there is real case - we found a record provided by a library system with this issue.

But i realize, that when preg_replace fails, the string is so bad, that it does not make sense to try it use somehow.... So I ended up with just throwing an exception.

[xmorave2]

@demiankatz I also updated the changelog and added a simple test

[demiankatz]

Thanks, @xmorave2!

[demiankatz]

Actually, I see one small problem -- this just needs vendor/bin/phing php-cs-fixer run on it to fix a minor issue with quotes. Once that is done and @EreMaijala approves of the changes, I can mint a new release so we can move forward with vufind-org/vufind#3276

[demiankatz]

I had a few free moments this morning, so I went ahead and applied the style fix. I think this is ready to merge, but I'll wait until @EreMaijala has a chance to look at it first.