Skip to content

Commit d1a02df

Browse files
Trofeomediaclaude
authored andcommitted
fix(h005): validate download arguments before any bank contact, use LocalDate
Review round 1 on the BTD download support. - Reject a partial or reversed date range in the EbicsDownloadParams constructor, the one place every caller passes through. A half range used to be dropped when the request was built, on the launcher's legacy path without even a warning; a reversed range is schema-valid and comes back as EBICS_NO_DOWNLOAD_DATA_AVAILABLE, indistinguishable from a genuinely empty period. - Check every launcher argument before the first environment read, keystore access or bank call. It ran after loadUser/createUser and after --ini/--hia/--hpb, so an incomplete --btd order could still fire an INI request first, and INI is one-shot at most banks. - Carry the report period as LocalDate instead of Date. A calendar day read out of an instant depends on the machine's timezone: a UTC-midnight Date becomes the previous day west of UTC. The Date-taking overloads are kept and now document that. Adds createDateRange(LocalDate, LocalDate). - Upper-case the EBICS code list values (--service, --scope, --option, --container) so --container zip no longer aborts. Message names such as camt.053 stay as given. Tests 31 -> 36. New guards were each seen failing first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 83129e0 commit d1a02df

7 files changed

Lines changed: 244 additions & 44 deletions

src/main/java/org/kopi/ebics/client/EbicsClient.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import org.kopi.ebics.session.OrderType;
5959
import org.kopi.ebics.session.Product;
6060
import org.kopi.ebics.utils.Constants;
61+
import org.kopi.ebics.xml.EbicsXmlFactory;
6162
import org.slf4j.Logger;
6263
import org.slf4j.LoggerFactory;
6364

@@ -446,10 +447,22 @@ public void fetchFile(File file, User user, Product product, EbicsOrderType orde
446447
}
447448
}
448449

450+
/**
451+
* Downloads a file for a report period.
452+
*
453+
* <p><b>A {@link Date} is an instant, the EBICS report period is a pair of calendar days.</b>
454+
* The calendar day is therefore read in the timezone of the machine running this code, so a
455+
* {@code Date} at UTC midnight becomes the previous day in any zone west of UTC. Prefer
456+
* {@link #fetchFile(File, User, Product, EbicsOrderType, EbicsDownloadParams, boolean)} with
457+
* {@link java.time.LocalDate} values, which has no timezone in it.
458+
*/
449459
public void fetchFile(File file, EbicsOrderType orderType, Date start, Date end) throws IOException,
450460
EbicsException {
451461
fetchFile(file, defaultUser, defaultProduct, orderType,
452-
EbicsDownloadParams.dateRangeOnly(start, end), false);
462+
EbicsDownloadParams.dateRangeOnly(
463+
start == null ? null : EbicsXmlFactory.toLocalDate(start),
464+
end == null ? null : EbicsXmlFactory.toLocalDate(end)),
465+
false);
453466
}
454467

455468
/**

src/main/java/org/kopi/ebics/client/EbicsDownloadParams.java

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package org.kopi.ebics.client;
22

3-
import java.util.Date;
3+
import java.time.LocalDate;
44

55
/**
66
* Service parameters for an EBICS 3.0 (H005) BTD download order.
@@ -9,6 +9,15 @@
99
* {@code BTDOrderParams/Service} block. With {@code serviceName} left {@code null}, only the
1010
* optional date range is applied and the legacy EBICS 2.x order type is kept, so existing
1111
* callers keep their behaviour.
12+
*
13+
* <p>The report period is a pair of calendar days ({@link LocalDate}), not instants: EBICS sends
14+
* it as {@code xs:date}, and a timezone in that position only creates off-by-one-day bugs.
15+
*
16+
* <p>The constructor rejects a partial or reversed range. Both would otherwise travel silently:
17+
* a half range is dropped when the request is built, and a reversed one is schema-valid and comes
18+
* back as "no data available", which is indistinguishable from a period that really was empty.
19+
* This is the single place every caller passes through, so the check lives here rather than in
20+
* each caller.
1221
*/
1322
public record EbicsDownloadParams(
1423
String serviceName,
@@ -17,11 +26,23 @@ public record EbicsDownloadParams(
1726
String messageName,
1827
String messageVersion,
1928
String containerType,
20-
Date startDate,
21-
Date endDate) {
29+
LocalDate startDate,
30+
LocalDate endDate) {
31+
32+
public EbicsDownloadParams {
33+
if ((startDate == null) != (endDate == null)) {
34+
throw new IllegalArgumentException(
35+
"startDate and endDate must be given together (--start/--end); a single one"
36+
+ " would be dropped from the bank request");
37+
}
38+
if (startDate != null && endDate.isBefore(startDate)) {
39+
throw new IllegalArgumentException(
40+
"endDate must not be before startDate, got " + startDate + " to " + endDate);
41+
}
42+
}
2243

2344
/** Date-range-only parameters for the legacy (non-BTD) download path. */
24-
public static EbicsDownloadParams dateRangeOnly(Date startDate, Date endDate) {
45+
public static EbicsDownloadParams dateRangeOnly(LocalDate startDate, LocalDate endDate) {
2546
if (startDate == null && endDate == null) {
2647
return null;
2748
}

src/main/java/org/kopi/ebics/client/ParameterizedEbicsClientLauncher.java

Lines changed: 49 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ public static void main(String[] args) throws Exception {
6868
return;
6969
}
7070

71+
// Every argument is checked before the first environment read, keystore access or bank
72+
// call. INI is one-shot at most banks: aborting on a missing --container after the INI
73+
// request has gone out would leave a half-initialised access behind.
74+
validateArguments(parsedArguments);
75+
7176
String passphrase = requiredEnv("EBICS_PASSWORD");
7277
String userId = requiredEnv("EBICS_USER_ID");
7378
String partnerId = requiredEnv("EBICS_PARTNER_ID");
@@ -162,10 +167,7 @@ public static void main(String[] args) throws Exception {
162167
user,
163168
product,
164169
orderType,
165-
EbicsDownloadParams.dateRangeOnly(
166-
parseDate(parsedArguments.startDate(), "--start"),
167-
parseDate(parsedArguments.endDate(), "--end")
168-
),
170+
legacyDownloadParams(parsedArguments),
169171
Boolean.parseBoolean(env("EBICS_TEST_MODE", "false"))
170172
);
171173
}
@@ -187,25 +189,50 @@ private static void printUsage() {
187189
System.out.println(usage);
188190
}
189191

192+
/**
193+
* Rejects every unusable argument combination before the program talks to anyone. Nothing here
194+
* touches the network, the filesystem or the environment.
195+
*/
196+
static void validateArguments(ParsedArguments parsedArguments) {
197+
if (parsedArguments.hasFlag("--btd")) {
198+
btdDownloadParams(parsedArguments);
199+
requireOutputPath(parsedArguments);
200+
} else {
201+
legacyDownloadParams(parsedArguments);
202+
}
203+
}
204+
190205
/**
191206
* Builds the EBICS 3.0 service parameters for {@code --btd}. Fails fast on a missing mandatory
192-
* value, so a half-filled order is never sent to the bank.
207+
* value, so a half-filled order is never sent to the bank. The date range pair itself is
208+
* checked by {@link EbicsDownloadParams}, which covers every other caller too.
193209
*/
194210
static EbicsDownloadParams btdDownloadParams(ParsedArguments parsedArguments) {
195-
// A half date range would be dropped silently further down, which is exactly how a
196-
// catch-up run loses the days it was supposed to fetch.
197-
if ((parsedArguments.startDate() == null) != (parsedArguments.endDate() == null)) {
198-
throw new IllegalArgumentException(
199-
"Options --start and --end must be given together, a single one is ignored"
200-
+ " by the bank request");
201-
}
202211
return new EbicsDownloadParams(
203-
requireOption(parsedArguments.serviceName(), "--service"),
204-
requireOption(parsedArguments.scope(), "--scope"),
205-
parsedArguments.option(),
212+
upperCase(requireOption(parsedArguments.serviceName(), "--service")),
213+
upperCase(requireOption(parsedArguments.scope(), "--scope")),
214+
upperCase(parsedArguments.option()),
206215
requireOption(parsedArguments.messageName(), "--msg-name"),
207216
requireOption(parsedArguments.messageVersion(), "--msg-version"),
208-
requireOption(parsedArguments.containerType(), "--container"),
217+
upperCase(requireOption(parsedArguments.containerType(), "--container")),
218+
parseDate(parsedArguments.startDate(), "--start"),
219+
parseDate(parsedArguments.endDate(), "--end")
220+
);
221+
}
222+
223+
/**
224+
* Service code, scope, service option and container type are EBICS code list values and are
225+
* always upper case. Message names like {@code camt.053} are not, and stay untouched.
226+
*/
227+
private static String upperCase(String value) {
228+
return value == null ? null : value.toUpperCase(Locale.ROOT);
229+
}
230+
231+
/**
232+
* Builds the date-range-only parameters of the legacy (EBICS 2.x) download path.
233+
*/
234+
static EbicsDownloadParams legacyDownloadParams(ParsedArguments parsedArguments) {
235+
return EbicsDownloadParams.dateRangeOnly(
209236
parseDate(parsedArguments.startDate(), "--start"),
210237
parseDate(parsedArguments.endDate(), "--end")
211238
);
@@ -223,14 +250,17 @@ private static String requireOption(String value, String option) {
223250
return normalized;
224251
}
225252

226-
private static Date parseDate(String value, String option) {
253+
/**
254+
* Parses a {@code YYYY-MM-DD} argument into a calendar day. No timezone is involved, so the
255+
* day the user typed is the day that reaches the bank, wherever the job runs.
256+
*/
257+
private static LocalDate parseDate(String value, String option) {
227258
String normalized = normalize(value);
228259
if (normalized == null) {
229260
return null;
230261
}
231262
try {
232-
return Date.from(LocalDate.parse(normalized)
233-
.atStartOfDay(ZoneId.systemDefault()).toInstant());
263+
return LocalDate.parse(normalized);
234264
} catch (DateTimeParseException e) {
235265
throw new IllegalArgumentException(
236266
"Option " + option + " expects a date as YYYY-MM-DD but was: " + normalized);

src/main/java/org/kopi/ebics/xml/DownloadInitializationRequestElement.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ public void buildInitialization() throws EbicsException {
115115
type.setStringValue(this.getType());
116116
StandardOrderParamsType standardOrderParamsType =
117117
EbicsXmlFactory.createStandardOrderParamsType();
118-
if (downloadParams != null
119-
&& downloadParams.startDate() != null && downloadParams.endDate() != null) {
118+
// EbicsDownloadParams guarantees the range is either absent or complete.
119+
if (downloadParams != null && downloadParams.startDate() != null) {
120120
standardOrderParamsType.setDateRange(EbicsXmlFactory.createDateRange(
121121
downloadParams.startDate(), downloadParams.endDate()));
122122
}

src/main/java/org/kopi/ebics/xml/EbicsXmlFactory.java

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
package org.kopi.ebics.xml;
2020

21+
import java.time.LocalDate;
2122
import java.time.ZoneId;
2223
import java.util.Calendar;
2324
import java.util.Date;
@@ -934,12 +935,15 @@ public static BTUParamsType createBTUParams(String serviceName, String scope, St
934935
* @param messageVersion the message version, e.g. {@code 08}
935936
* @param containerType the container type ({@code XML}, {@code ZIP} or {@code SVC});
936937
* may be {@code null}
937-
* @param start the start of the requested report period; may be {@code null}
938-
* @param end the end of the requested report period; may be {@code null}
938+
* @param start the first calendar day of the requested report period; may be
939+
* {@code null}
940+
* @param end the last calendar day of the requested report period; may be
941+
* {@code null}
939942
* @return the <code>BTDParamsType</code> XML object
940943
*/
941944
public static BTDParamsType createBTDParams(String serviceName, String scope, String option,
942-
String messageName, String messageVersion, String containerType, Date start, Date end) {
945+
String messageName, String messageVersion, String containerType,
946+
LocalDate start, LocalDate end) {
943947
var type = BTDParamsType.Factory.newInstance();
944948
var service = type.addNewService();
945949
service.setServiceName(serviceName);
@@ -972,14 +976,15 @@ public static BTDParamsType createBTDParams(String serviceName, String scope, St
972976
}
973977

974978
/**
975-
* Converts a date into an <code>xs:date</code> value without a timezone offset. Passing a
976-
* {@link Calendar} instead would make XMLBeans append the local offset (e.g.
977-
* {@code 2026-08-10+02:00}), which shifts the reported day for a bank in another timezone.
979+
* Converts a calendar day into an <code>xs:date</code> value. No timezone is involved in
980+
* either direction: setting a {@link Calendar} would make XMLBeans append the local offset
981+
* (e.g. {@code 2026-08-10+02:00}), which shifts the reported day for a bank in another
982+
* timezone, and converting through an instant would make the day itself depend on the
983+
* machine's zone.
978984
*/
979-
private static DateType toXmlDate(Date date) {
985+
private static DateType toXmlDate(LocalDate date) {
980986
var value = DateType.Factory.newInstance();
981-
value.setStringValue(
982-
date.toInstant().atZone(ZoneId.systemDefault()).toLocalDate().toString());
987+
value.setStringValue(date.toString());
983988
return value;
984989
}
985990

@@ -1034,13 +1039,29 @@ public static StandardOrderParamsType createStandardOrderParamsType() {
10341039
}
10351040

10361041
/**
1037-
* Creates a new <code>DateRange</code> XML object
1042+
* Creates a new <code>DateRange</code> XML object.
1043+
*
1044+
* <p><b>A {@link Date} is an instant, the EBICS date range is a pair of calendar days.</b>
1045+
* The calendar day is therefore taken in the timezone of the machine running this code: a
1046+
* {@code Date} at UTC midnight becomes the previous day in any zone west of UTC. Prefer
1047+
* {@link #createDateRange(LocalDate, LocalDate)} — that overload has no timezone in it.
10381048
*
10391049
* @param start the start range
10401050
* @param end the end range
10411051
* @return the <code>DateRange</code> XML object
10421052
*/
10431053
public static StandardOrderParamsType.DateRange createDateRange(Date start, Date end) {
1054+
return createDateRange(toLocalDate(start), toLocalDate(end));
1055+
}
1056+
1057+
/**
1058+
* Creates a new <code>DateRange</code> XML object from two calendar days.
1059+
*
1060+
* @param start the first day of the range
1061+
* @param end the last day of the range
1062+
* @return the <code>DateRange</code> XML object
1063+
*/
1064+
public static StandardOrderParamsType.DateRange createDateRange(LocalDate start, LocalDate end) {
10441065
StandardOrderParamsType.DateRange newDateRange = StandardOrderParamsType.DateRange.Factory.newInstance();
10451066

10461067
newDateRange.xsetStart(toXmlDate(start));
@@ -1049,6 +1070,14 @@ public static StandardOrderParamsType.DateRange createDateRange(Date start, Date
10491070
return newDateRange;
10501071
}
10511072

1073+
/**
1074+
* Reads the calendar day out of an instant, in the timezone of this machine. Only for the
1075+
* {@link Date}-based compatibility overloads; anything new should carry a {@link LocalDate}.
1076+
*/
1077+
public static LocalDate toLocalDate(Date date) {
1078+
return date.toInstant().atZone(ZoneId.systemDefault()).toLocalDate();
1079+
}
1080+
10521081
// /**
10531082
// * Creates a new <code>FileFormatType</code> XML object
10541083
// *

0 commit comments

Comments
 (0)