Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with Resource Parser #8091

Merged
merged 5 commits into from
Jun 6, 2023
Merged

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented May 31, 2023

We have seen reports of the following error/warning when building Maui or Android projects.

XAGRDA7005
System.FormatException: Could not find any recognizable digits.  
at System.ParseNumbers.StringToInt(String s, Int32 radix, Int32 flags, Int32* currPos)   
at System.Convert.ToInt32(String value, Int32 fromBase)   
at Xamarin.Android.Tasks.RtxtParser.ProcessRtxtFile(String file, IList`1 result)   
at Xamarin.Android.Tasks.RtxtParser.Parse(String file, TaskLoggingHelper logger, Dictionary`2 mapping)   
at Xamarin.Android.Tasks.GenerateResourceDesignerAssembly.Run(DirectoryAssemblyResolver res)   
at Xamarin.Android.Tasks.GenerateResourceDesignerAssembly.RunTask()   
at Microsoft.Android.Build.Tasks.AndroidTask.Execute() in /Users/runner/work/1/s/xamarin-android/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AndroidTask.cs:line 25

Our initial thoughts were that we were trying to parse an empty R.txt file. However this did not seem to pan out.
Further investigation found this line in an R.txt file

int id maximum number 0x7f037465

This is very odd, since id values should NOT contain spaces.
It turns out this is caused by the following code

<resources xmlns:ns1="http://schemas.android.com/tools" xmlns:ns2="urn:oasis:names:tc:xliff:document:1.2">
<string name="translatable_text" translatable="false"><ns2:g example="999" id="maximum number">%1$d</ns2:g><ns2:g example="+" id="suffix">%2$s</ns2:g></string>
</resources>

This is from com.google.android.material.material.aar. It turns out that you can have additional namespaces in the resource xml files. aapt2 ignores these additional namespaces but OUR managed parser does not.
As a result the id="maximum number" attribute gets picked up as a valid id value.

The fix in this case is to check if the containing element has a namespace. If it does then make sure it is the android resource namespace. ALL other namespaces will be ignored. We also added a new Regex to check for invalid characters just incase the Namespace check allows some entires though.

@dellis1972 dellis1972 marked this pull request as ready for review June 2, 2023 15:13
@dellis1972 dellis1972 requested review from jonpryor and jonathanpeppers and removed request for jonathanpeppers June 2, 2023 15:13
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good if CI ends up green. 👍

@@ -63,6 +64,7 @@ public class RtxtParser {
static readonly char[] EmptyChar = new char [] { ' ' };
static readonly char[] CurlyBracketsChar = new char [] { '{', '}' };
static readonly char[] CommaChar = new char [] { ',' };
static readonly Regex ValidChars = new Regex (@"([^a-f0-9x, \{\}])+", RegexOptions.Compiled | RegexOptions.IgnoreCase);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point, should we start using the faster Regex source generator?

https://learn.microsoft.com/dotnet/standard/base-types/regular-expression-source-generators

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. We can do that in one pass I think. Maybe not in this PR though :D

@jonpryor jonpryor merged commit ffbe422 into dotnet:main Jun 6, 2023
grendello added a commit to grendello/xamarin-android that referenced this pull request Jun 7, 2023
* main:
  [ci] Shut down dotnet processes before retrying failed unit tests. (dotnet#8107)
  Bump to dotnet/installer@18dc2cf1 8.0.100-preview.6.23305.2
  [Xamarin.Android.Build.Tasks] Ignore non-Android XML resources (dotnet#8091)
  [build] Ignore CA1305 in more projects (dotnet#8110)
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants