nissl-lab / npoi

a .NET library that can read/write Office formats without Microsoft Office installed. No COM+, no interop.
Apache License 2.0
5.65k stars 1.42k forks source link

nsid is generated incorrectly for abstract num #1358

Closed hahschaa closed 2 months ago

hahschaa commented 2 months ago

NPOI 2.7.0

File Type

Reproduce Steps

The issue is illustrated by the following example code: In the top example the lower bytes are used, which change between successive calls. The bottom example uses the higher bytes, which will not change quick enough.

var a = new byte[4];
var b = new byte[4];

// Lower bytes are different.
Array.Copy(BitConverter.GetBytes(DateTime.Now.Ticks), 0, a, 0, 4);
Array.Copy(BitConverter.GetBytes(DateTime.Now.Ticks), 0, b, 0, 4);
Debug.WriteLine(BitConverter.ToString(a));
Debug.WriteLine(BitConverter.ToString(b));

// Higher bytes are the same.
Array.Copy(BitConverter.GetBytes(DateTime.Now.Ticks), 4, a, 0, 4);
Array.Copy(BitConverter.GetBytes(DateTime.Now.Ticks), 4, b, 0, 4);
Debug.WriteLine(BitConverter.ToString(a));
Debug.WriteLine(BitConverter.ToString(b));

Issue Description

I ran into an issue when creating abstract num instances. The constructor will assign an nsid based on the current tick count, but the wrong 4 bytes are used, which result in successive identical identifiers.

The issue can be found here: https://github.com/nissl-lab/npoi/blob/master/OpenXmlFormats/Wordprocessing/Numbering.cs#L974

Bykiev commented 2 months ago

Hi, would you like to contribute?

hahschaa commented 2 months ago

Sorry, no, I do not have time to be a contributor. I would like to fix this bug for you though, but the only thing you have to do is to change the 4 into a 0. I cannot push my change to your repo since I do not have permissions.

tonyqus commented 2 months ago

I cannot push my change to your repo since I do not have permissions.

This is how Pull Request works. You don't need permission for commit in one repo.

Anyway, if you don't have time, it's fine.

hahschaa commented 2 months ago

I mean that I have a local branch with the fix, but that I cannot push it:

git push --set-upstream origin bugfix/incorrect-nsid-for-abstract-num

ERROR: Permission to nissl-lab/npoi.git denied to hahschaa.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
Bykiev commented 2 months ago

I mean that I have a local branch with the fix, but that I cannot push it:

git push --set-upstream origin bugfix/incorrect-nsid-for-abstract-num

ERROR: Permission to nissl-lab/npoi.git denied to hahschaa.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

You should fork the repo, make changes in your fork and then create a PR to main branch

tonyqus commented 2 months ago

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork

hahschaa commented 2 months ago

Like I said, it's just a single character to change, so I think creating a fork just for that is a bit much. But here you go: https://github.com/nissl-lab/npoi/pull/1362