il-repack icon indicating copy to clipboard operation
il-repack copied to clipboard

Bug: ILRepack creates corrupted resources by merging NET Core3.1 WindowsForms Applications

Open ymalich opened this issue 3 years ago • 6 comments

Hello, I've just found the bug by merging class libraries with a WindowsForms Application targeted to NET Core3.1. The integrated image resources can't be loaded anymore. The exception is being thrown "Unhandled exception. System.Runtime.Serialization.SerializationException: The input stream is not a valid binary format. The starting contents (in bytes) are: 04-AE-01-89-50-4E-47-0D-0A-1A-0A-00-00-00-0D-49-48 ...". I've created the small test project with all the binaries in order you can reproduce the problem easiely. Please find attached. WindowsFormsTestCoreApp.zip

ymalich avatar Oct 03 '20 10:10 ymalich

I've found where the problem is. The System.Resources.ResourceWriter is used by ILRepack to re-create resources. It always generates .NET Framework resource reader type ("System.Resources.ResourceReader, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089#System.Resources.RuntimeResourceSet") in the packed resource byte stream. The Core .NET applications use another resource reader type to read resources - "System.Resources.Extensions.DeserializingResourceReader, System.Resources.Extensions, Version=4.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51". I've just fixed the problem locally by passing source resource byte block to output stream without re-creation. I'll make tests for this case and create the pull-request.

BTW. I just merged the latest version Mono.Cecil into the version, used by ILRepack (it supports netstandard2.0), converted ILRepack to SDK-project format with net472 and NetCore3.1 output and made several code adaptations to the new Mono.Cecil version. Should I create the pull request for this upgrade?

ymalich avatar Oct 21 '20 21:10 ymalich

This project does not look alive. Migrating to cecil in #236 is sitting in review for 2.5 years. I'd say PR improving current state is good - even if it won't be merged, it gives ideas what can be done in different situations to others. And project desperately needs some maintainer / well maintained fork.

iskiselev avatar Jan 05 '21 05:01 iskiselev

I see. Would be great to read the opinion of @gluck first, if he would like to give away the ownership to community. The most difficult question after that is - who would like to be the maintainer? I'm currently missing some knowledge in this area, so I'm not ready to be the maintainer, who could accept pull-request from others.

ymalich avatar Jan 05 '21 20:01 ymalich

@iskiselev I am handling some of the maintainer tasks, though I didn't manage to review all the remaining stuff 😓

That PR was particularly tricky because it was actually breaking some of our internal apps with that upgrade. And as you posted there seem to be some other issues. However, we will need that update, so I will try to expedite the merging. Maybe we will release 3.0 as an alpha version with support for .NET Core

timotei avatar Feb 22 '21 10:02 timotei

Actually I'm keeping the issue open to investigate whether a better solution would be to use the same serializer as the original assembly. If the original resource is Core, use the Core serializer.

KirillOsenkov avatar Jan 01 '24 02:01 KirillOsenkov

Hello, the new version 2.0.21 merges my WinForms application assemblies correctly, without exceptions. I've testet with .NET 6.0, 7.0 and 8.0. Best regards,Yury 

ymalich avatar Jan 01 '24 13:01 ymalich