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

OaIdl.DATE(Date javaDate) sets wrong date in some cases #1460

Closed
eranl opened this issue Aug 22, 2022 · 9 comments
Closed

OaIdl.DATE(Date javaDate) sets wrong date in some cases #1460

eranl opened this issue Aug 22, 2022 · 9 comments

Comments

@eranl
Copy link

eranl commented Aug 22, 2022

  1. Version of JNA and related jars: 5.12.1

  2. Version and vendor of the java virtual machine:
    IMPLEMENTOR="Oracle Corporation"
    JAVA_VERSION="17.0.1"

  3. Operating system: Windows 7

  4. System architecture (CPU type, bitness of the JVM): x86_64

  5. Complete description of the problem:
    When com.sun.jna.platform.win32.OaIdl.DATE(Date javaDate) is called with a javaDate whose time part converted to local time is between midnight and 1am, while DST (60-minute offset) is on, the date portion of the resulting DATE object is of the previous day, while the time portion is correct, which means that the DATE object is 24 hours behind.

  6. Steps to reproduce:
    Simple test: with DST on, instantiate one OaIdl.DATE(Date javaDate) with a javaDate having the time 12:30am local time and another with 1:30am local time, with the same date, and you'll see that the resulting DATEs have different integralParts.

@dbwiddis
Copy link
Contributor

Java time has no concept of DST, so the first hour of the day ends up being the "last hour" of a 24 hour period with reference to the epoch, as assumed in this calculation:

double msSinceOrigin = javaDate.getTime() - DATE_OFFSET;
double daysAsFract = msSinceOrigin / MICRO_SECONDS_PER_DAY;

However, this may be by design, per the Windows docs for DATE:

The following points should be noted when working with these date and time formats in Automation:

Dates are specified in local time; synchronization must be performed manually when working with dates in different time zones.

The date types do not account for Daylight Savings Time.

One would reason based on that last sentence that it is an error to use a DST-impacted Java Date in this constructor.

@eranl
Copy link
Author

eranl commented Aug 23, 2022

Any reason not to use ZonedDateTime for the conversion from UTC to local time, rather than the above calculation?

@dbwiddis
Copy link
Contributor

Any reason not to use ZonedDateTime

It requires JDK 8. We maintain (at least in 5.X series) compatibility with JDK 6.

@dbwiddis
Copy link
Contributor

@eranl
Copy link
Author

eranl commented Aug 24, 2022

Thanks for the info. Very interesting.

Here's the latest version: https://svn.apache.org/viewvc/poi/trunk/poi/src/main/java/org/apache/poi/ss/usermodel/DateUtil.java?view=markup

Can I contribute a fix based on this?

@dbwiddis
Copy link
Contributor

Please do.

Also a test case failing in the existing setup and passing with the fix would be helpful.

@matthiasblaesing
Copy link
Member

matthiasblaesing commented Aug 24, 2022 via email

@dbwiddis
Copy link
Contributor

Agree that you cannot copy, but there are other factors to consider (POI is based on Jan 1 1900 epoch, DATE is based on Dec 30 1899) so assuming your "based on" means looking at how Calendar was used and doing something similar with the Win32 API base date, I think you'll be fine.

@matthiasblaesing
Copy link
Member

Fixed via #1461 (merged via 09d3371). Thank you again.

benkard added a commit to benkard/mulkcms2 that referenced this issue Jan 14, 2023
This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [net.java.dev.jna:jna](https://github.com/java-native-access/jna) | compile | minor | `5.12.1` -> `5.13.0` |

---

### Release Notes

<details>
<summary>java-native-access/jna</summary>

### [`v5.13.0`](https://github.com/java-native-access/jna/blob/HEAD/CHANGES.md#Release-5130)

[Compare Source](java-native-access/jna@5.12.1...5.13.0)

\================

## Features

-   [#&#8203;1454](java-native-access/jna#1454): Add `c.s.j.p.win32.Psapi.QueryWorkingSetEx` and associated Types - [@&#8203;crain-32](https://github.com/Crain-32).
-   [#&#8203;1459](java-native-access/jna#1459): Add `VirtualLock` and `VirtualUnlock` in `c.s.j.p.win32.Kernel32` - [@&#8203;matthiasblaesing](https://github.com/matthiasblaesing).
-   [#&#8203;1471](java-native-access/jna#1471): Add `c.s.j.p.win32.Advapi32Util#isCurrentProcessElevated` and associated Types - [@&#8203;dbwiddis](https://github.com/dbwiddis).
-   [#&#8203;1474](java-native-access/jna#1474): Add `c.s.j.p.win32.WbemCli#IWbemClassObject.IWbemQualifierSet`, `IWbemServices.GetObject`, `IWbemContext.SetValue` and associated methods - [@&#8203;rchateauneu](https://github.com/rchateauneu).
-   [#&#8203;1482](java-native-access/jna#1482): Add multilingual support of `Kernel32Util.formatMessage` - [@&#8203;overpathz](https://github.com/overpathz).
-   [#&#8203;1490](java-native-access/jna#1490): Adds support for a custom `SymbolProvider` in `NativeLibrary` & `Library` - [@&#8203;soywiz](https://github.com/soywiz).
-   [#&#8203;1491](java-native-access/jna#1491): Update libffi to v3.4.4  - [@&#8203;matthiasblaesing](https://github.com/matthiasblaesing).
-   [#&#8203;1487](java-native-access/jna#1487): Add 'uses' information to OSGI metadata in MANIFEST.MF to improve stability of package resolution - [@&#8203;sratz](https://github.com/sratz).

## Bug Fixes

-   [#&#8203;1452](java-native-access/jna#1452): Fix memory allocation/handling for error message generation in native library code (`dispatch.c`) - [@&#8203;matthiasblaesing](https://github.com/matthiasblaesing).
-   [#&#8203;1460](java-native-access/jna#1460): Fix win32 variant date conversion in DST offest window and with millisecond values - [@&#8203;eranl](https://github.com/eranl).
-   [#&#8203;1472](java-native-access/jna#1472): Fix incorrect bitmask in `c.s.j.Pointer#createConstant(int)` - [@&#8203;dbwiddis](https://github.com/dbwiddis).
-   [#&#8203;1481](java-native-access/jna#1481): Fix NPE in NativeLibrary when unpacking from classpath is disabled - [@&#8203;trespasserw](https://github.com/trespasserw).
-   [#&#8203;1489](java-native-access/jna#1489): Fixes typo in `OpenGL32Util#wglGetProcAddress`, instead of parameter `procName` the hardcoded value `wglEnumGpusNV` was used - [@&#8203;soywiz](https://github.com/soywiz).

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants