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

nsid is generated incorrectly for abstract num #1358

Closed
1 of 5 tasks
hahschaa opened this issue May 30, 2024 · 7 comments
Closed
1 of 5 tasks

nsid is generated incorrectly for abstract num #1358

hahschaa opened this issue May 30, 2024 · 7 comments

Comments

@hahschaa
Copy link

NPOI 2.7.0

File Type

  • XLSX
  • XLS
  • DOCX
  • XLSM
  • OTHER

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

@hahschaa hahschaa added the bug label May 30, 2024
@Bykiev
Copy link
Collaborator

Bykiev commented May 30, 2024

Hi, would you like to contribute?

@hahschaa
Copy link
Author

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
Copy link
Member

tonyqus commented May 30, 2024

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.

@tonyqus tonyqus added the docx label May 30, 2024
@tonyqus tonyqus added this to the NPOI 2.7.2 milestone May 30, 2024
@hahschaa
Copy link
Author

hahschaa commented May 30, 2024

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
Copy link
Collaborator

Bykiev commented May 30, 2024

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

@hahschaa
Copy link
Author

hahschaa commented Jun 4, 2024

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:
#1362

tonyqus added a commit that referenced this issue Jun 6, 2024
@tonyqus tonyqus closed this as completed Jun 6, 2024
@tonyqus tonyqus modified the milestones: NPOI 2.7.2, NPOI 2.7.1 Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants