Extra IDs larger than `short.MaxValue` not supported in `ZipExtraData`

[rougemeilland]

Steps to reproduce

1. Define a class of extra field with tag ID 0xe57aby implementing TaggedData.
2. Pass the implemented class instance to the ZipExtraData.AddEntry method.

Please refer to the source code of the program to reproduce this problem.

Expected behavior

The extra field is successfully added to the ZipExtraData object. Then I can refer to the extra field using something like the ZipExtraData.GetData method.

Actual behavior

In the ZipExtraData.AddEntry method, a System.ArgumentOutOfRangeException exception occurs.

Version of SharpZipLib

SharpZipLib 1.3.2

Obtained from (only keep the relevant lines)

• Package installed using NuGet

Supplementary information

Some explanation is needed as to why this problem occurred.

What I originally wanted to do was parse an unfamiliar extra field with a tag ID of 0xe57a. I found the tag in the ZIP file I have.

The specification of the tag is unclear, but according to "https://gnqg.hatenablog.com/entry/2016/09/11/155033", it seems to explicitly specify the code page of the entry name. It seems to have been used in an application called "ALZip" in the past.

Anyway, I wrote the code to try to get 0xe57a extra field from that ZIP file. However, even though ZipEntry.ExtraData contains that extra field, for some reason the ZipExtraData.GetData method returns null.

I thought it was weird and tried to write a sample program to fix the problem, but in fact I got an exception in ZipExtraData.AddEntry which was executed before the ZipExtraData.GetData method.

I read the source code of SharpZipLib/src/ICSharpCode.SharpZipLib/Zip/ZipExtraData.cs.
As a result, it seemed that ZipExtraData.AddEntry would raise an exception if ITaggedData.TagID returned a value greater than or equal to 0x8000.
Also, the reason why the ZipExtraData.GetData method returned null is that the value of ITaggedData.TagID was expanded to int and treated as a negative value (0xffffe57a), and the ReadShortInternal method I think that the cause is that when reading the tag ID from the byte array, the operation is performed as an unsigned integer and the value (0x0000e57a) is returned as an int type.

And the next thing to think about is, "Is a 16-bit integer greater than or equal to 0x8000 valid as a tag ID?"
I think that "a 16-bit integer of 0x8000 or more is also valid as a tag ID" for the following reasons.

• The list of well-known extra fields (proginfo/extrald.txt) contains 0xfb4a (SMS / QDOS).
• Although it is specified that the tag ID is a 16-bit integer, there is no specification indicating whether it should be in the range of 0x0000 to 0x7fff or whether it should be treated as a signed integer. (Maybe I just don't know y well ...)

Source code of sample program

using ICSharpCode.SharpZipLib.Zip;
using System;
using System.Text;

namespace Experiment
{
    class Program
    {
        private class SampleExtraField
            : ITaggedData
        {
            public short TagID
            {
                get
                {
                    // Attempting to cast 0xe57a to short results in a compilation error "Compiler Error CS0221", so we are using the unchecked syntax here.
                    unchecked
                    {
                        return (short)0xe57a;
                    }
                }
            }

            public byte[] GetData()
            {
                return BitConverter.GetBytes(CodePage);
            }

            public void SetData(byte[] data, int offset, int count)
            {
                CodePage = BitConverter.ToInt32(data, offset);
            }

            public Int32 CodePage { get; set; }
        }

        static void Main(string[] args)
        {
            using (var extraData = new ZipExtraData(new byte[0]))
            {
                var now = DateTime.UtcNow;
                extraData.AddEntry(new NTTaggedData
                {
                    LastModificationTime = now,
                    LastAccessTime = now,
                    CreateTime = now,
                });

                if (extraData.GetData<NTTaggedData>() != null)
                    Console.WriteLine("OK (tag id = 0x000a)");
                else
                    Console.WriteLine("NG (tag id = 0x000a)");


                extraData.AddEntry(new SampleExtraField // <= I got an exception here
                {
                    CodePage = Encoding.Default.CodePage
                });
                if (extraData.GetData<SampleExtraField>() != null)
                    Console.WriteLine("OK (tag id = 0xe57a)");
                else
                    Console.WriteLine("NG (tag id = 0xe57a)");

            }
            Console.ReadLine();
        }
    }
}

[piksel]

Yeah, this seems like an overthought (or the problem just hasn't surfaced, so there were no incentive to support it before?)

And the next thing to think about is, "Is a 16-bit integer greater than or equal to 0x8000 valid as a tag ID?"
I think that "a 16-bit integer of 0x8000 or more is also valid as a tag ID" for the following reasons.

Indeed, the Extra ID is defined as 2 bytes (https://p1k.se/appnote.md#s45-extensible-data-fields), which indicates that any two bytes would be valid, whether they would be a negative number when treated as a signed short.

[Numpsy]

I think I hit that size issue when looking at #470 , though that change hasn't gone anywhere further.

[piksel]

The only reason I can think of why this wasn't done before is using -1 to mean N/A, but then again, when would that ever be required? And even if it's needed somewhere, we could just use 0 as it's reserved but doesn't map to anything (perhaps even reserved for this purpose?)

[piksel]

Should be fixed as per #669.