Handle faulty po files

[MatteoPiovanelli]

I was testing things for localized content, in the 1.10.x branch.
In a French .po file for a module I have there was an error. A line is

msgctxt "

This is clearly wrong.

The behaviour when opening a French content in the front-end was to throw out an Exception page with the whole call stack.

The fault was happening in the TrimQuote(string) method in
Orchard/src/Orchard/Localization/Services/LocalizationStreamParser.cs
(https://github.com/OrchardCMS/Orchard/blob/1.10.x/src/Orchard/Localization/Services/LocalizationStreamParser.cs)

That faulty line in my po file results in the TrimQuote method being passed str = "\"" as parameter. This verifies the condition, but causes an exception when calling Substring(int, int), because str.Length == 1.

I am not sure how that line ended in the po file, but it makes me think that it's not safe to assume, in the code, that .po files will always be well formatted. At the very least, errors in the .po file should be handled in such a way that avoids crashing the application.

[MpDzik]

I'll try to fix this over the weekend.

[MatteoPiovanelli]

@MpDzik thanks.
I realize the error is in my po file (so I went and fixed it) but I still feel the localization service should be more robust there.

[MpDzik]

@MatteoPiovanelli-Laser I completely agree with you. I am just wondering how to handle this. I could probably treat this case as an empty string or alternatively throw an exception with a more reasonable message.

[MatteoPiovanelli]

I am wondering if using string.Remove(int) and string.Remove(int, int) may not be more robust than stirng.Substring(int, int). It would likely impact performance negatively.
Alternatively, have that exception there handled as a "NotFatal" exception somehow?

[MpDzik]

I think it will be sufficient to check the length of the string, as this is a corner case when the string is too short. I guess it would be best to write an error (or maybe even warning) to the log and handle this case as an empty string.

[MatteoPiovanelli]

That's probably right

[MpDzik]

Created pull request #7401

[MatteoPiovanelli]

saw this was merged in 1.10.x.
Thanks.