Skip to content

Commit ebdca9d

Browse files
committed
HtmlFileInput.reset() / HtmlFileInput.isValid() fixed;
data handling also improved and many more tests added
1 parent 211cd67 commit ebdca9d

5 files changed

Lines changed: 255 additions & 17 deletions

File tree

src/changes/changes.xml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@
88

99
<body>
1010
<release version="5.4.0" date="August xx, 2026" description="Firefox 153, Bugfixes">
11+
<action type="fix" dev="rbri">
12+
HtmlFileInput.reset(): a form reset no longer tries to reconstruct a fake File from the
13+
'value' attribute; it now simply clears the selected files, without firing a change event.
14+
</action>
15+
<action type="fix" dev="rbri">
16+
HtmlFileInput.isValid(): a disabled file input is no longer incorrectly reported as invalid
17+
by checkValidity() when 'required' is set and no file is selected
18+
</action>
1119
<action type="add" dev="rbri">
1220
Method reportValidity() support to various form controls added.
1321
</action>

src/main/java/org/htmlunit/HttpWebConnection.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,7 @@ void buildFilePart(final KeyDataPair pairWithFile, final MultipartEntityBuilder
467467
filename = pairWithFile.getValue();
468468
}
469469

470-
builder.addBinaryBody(pairWithFile.getName(), new ByteArrayInputStream(data),
471-
contentType, filename);
470+
builder.addBinaryBody(pairWithFile.getName(), data, contentType, filename);
472471
return;
473472
}
474473

src/main/java/org/htmlunit/html/HtmlFileInput.java

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,28 @@ public NameValuePair[] getSubmitNameValuePairs() {
107107

108108
final List<NameValuePair> list = new ArrayList<>();
109109
for (final File file : files_) {
110-
String contentType;
111-
if (contentType_ == null) {
112-
contentType = getPage().getWebClient().getBrowserVersion().getUploadMimeType(file);
113-
if (StringUtils.isEmptyOrNull(contentType)) {
114-
contentType = MimeType.APPLICATION_OCTET_STREAM;
110+
final Charset charset = getPage().getCharset();
111+
112+
final KeyDataPair keyDataPair;
113+
if (data_ == null) {
114+
String contentType;
115+
if (contentType_ == null) {
116+
contentType = getPage().getWebClient().getBrowserVersion().getUploadMimeType(file);
117+
if (StringUtils.isEmptyOrNull(contentType)) {
118+
contentType = MimeType.APPLICATION_OCTET_STREAM;
119+
}
115120
}
121+
else {
122+
contentType = contentType_;
123+
}
124+
125+
keyDataPair = new KeyDataPair(getNameAttribute(), file, null, contentType, charset);
116126
}
117127
else {
118-
contentType = contentType_;
128+
keyDataPair = new KeyDataPair(getNameAttribute(), null, file.getName(),
129+
MimeType.APPLICATION_OCTET_STREAM, charset);
130+
keyDataPair.setData(data_);
119131
}
120-
final Charset charset = getPage().getCharset();
121-
final KeyDataPair keyDataPair = new KeyDataPair(getNameAttribute(), file, null, contentType, charset);
122-
keyDataPair.setData(data_);
123132
list.add(keyDataPair);
124133
}
125134
return list.toArray(new NameValuePair[0]);
@@ -279,12 +288,35 @@ public File[] getFiles() {
279288
return files_;
280289
}
281290

291+
/**
292+
* {@inheritDoc}
293+
* Per spec, the reset algorithm for a file upload control is special-cased:
294+
* it simply empties the list of selected files. Unlike other input types,
295+
* there is no "restore to the value attribute" step at all -- a browser can
296+
* never resurrect a specific file selection from markup, for the same
297+
* security reason the 'value' attribute is unsettable and fake-pathed in
298+
* the first place. Deliberately does NOT call super.reset() (which would
299+
* try to reconstruct a fake File from the 'value' attribute via setValue())
300+
* and deliberately does NOT fire a change event, matching the same
301+
* "reset fires a reset event, not a change event" principle already applied
302+
* to HtmlInput/HtmlTextArea.
303+
* @see SubmittableElement#reset()
304+
*/
305+
@Override
306+
public void reset() {
307+
files_ = new File[0];
308+
}
309+
282310
/**
283311
* Returns whether this element satisfies all form validation constraints set.
284312
* @return whether this element satisfies all form validation constraints set
285313
*/
286314
@Override
287315
public boolean isValid() {
316+
if (isDisabled()) {
317+
return true;
318+
}
319+
288320
return isCustomValidityValid()
289321
&& (!isRequiredSupported()
290322
|| ATTRIBUTE_NOT_DEFINED == getAttributeDirect("required")

src/test/java/org/htmlunit/html/HtmlFileInput2Test.java

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package org.htmlunit.html;
1616

1717
import static java.nio.charset.StandardCharsets.UTF_8;
18+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
1819

1920
import java.io.BufferedReader;
2021
import java.io.ByteArrayOutputStream;
@@ -762,4 +763,74 @@ public void clearFromJava() throws Exception {
762763
file.setValue("");
763764
assertEquals(0, file.getFiles().length);
764765
}
766+
767+
/**
768+
* Methods setData()/getData() are pure Java API (not exposed to JS at all, since
769+
* real browsers have no equivalent) and are completely untested in this
770+
* file. Verifies the getter/setter round-trip and that in-memory data is
771+
* actually used during submission instead of reading the file from disk.
772+
* @throws Exception if an error occurs
773+
*/
774+
@Test
775+
public void setDataIsUsedInsteadOfFileContentOnSubmit() throws Exception {
776+
final String htmlContent = DOCTYPE_HTML
777+
+ "<html>\n"
778+
+ "<body>\n"
779+
+ "<form action='upload2' method='post' enctype='multipart/form-data'>\n"
780+
+ " <input name='myInput' type='file' id='myInput'><br>\n"
781+
+ " <input type='submit' value='Upload' id='mySubmit'>\n"
782+
+ "</form>\n"
783+
+ "</body></html>\n";
784+
getMockWebConnection().setDefaultResponse("hello", MimeType.TEXT_PLAIN);
785+
786+
final HtmlPage page = loadPage(htmlContent);
787+
final HtmlFileInput input = (HtmlFileInput) page.getElementById("myInput");
788+
789+
final File tmpFile = File.createTempFile("htmlunit-test", ".txt");
790+
try {
791+
org.apache.commons.io.FileUtils.writeStringToFile(tmpFile, "content on disk", "UTF-8");
792+
input.setFiles(tmpFile);
793+
794+
input.setData("in-memory content".getBytes("UTF-8"));
795+
page.getElementById("mySubmit").click();
796+
}
797+
finally {
798+
assertTrue(tmpFile.delete());
799+
}
800+
801+
final String requestBody = getMockWebConnection().getLastWebRequest().getRequestBody();
802+
assertTrue(requestBody, requestBody.contains("in-memory content"));
803+
assertFalse(requestBody, requestBody.contains("content on disk"));
804+
}
805+
806+
/**
807+
* Plain Java-level round trip for setData()/getData()/setContentType()/
808+
* getContentType().
809+
* @throws Exception if an error occurs
810+
*/
811+
@Test
812+
public void setDataGetDataSetContentTypeRoundTrip() throws Exception {
813+
final String htmlContent = DOCTYPE_HTML
814+
+ "<html><body>\n"
815+
+ "<form id='form1'>\n"
816+
+ " <input type='file' id='f' name='f'>\n"
817+
+ "</form>\n"
818+
+ "</body></html>";
819+
820+
final HtmlPage page = loadPage(htmlContent);
821+
final HtmlFileInput input = (HtmlFileInput) page.getElementById("f");
822+
823+
assertNull(input.getData());
824+
assertNull(input.getContentType());
825+
826+
final byte[] data = "hello".getBytes("UTF-8");
827+
input.setData(data);
828+
input.setContentType("text/custom");
829+
830+
assertArrayEquals(data, input.getData());
831+
assertEquals("text/custom", input.getContentType());
832+
833+
input.setContentType(null);
834+
assertNull(input.getContentType());
835+
}
765836
}

src/test/java/org/htmlunit/html/HtmlFileInputTest.java

Lines changed: 134 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -339,21 +339,21 @@ public void resetByClick() throws Exception {
339339
+ " var file = document.getElementById('testId');\n"
340340
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
341341

342-
+ " document.getElementById('testReset').click;\n"
342+
+ " document.getElementById('testReset').click();\n"
343343
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
344344

345345
+ " try{\n"
346346
+ " file.value = 'newValue';\n"
347347
+ " } catch(e) { logEx(e); }\n"
348348
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
349349

350-
+ " document.getElementById('testReset').click;\n"
350+
+ " document.getElementById('testReset').click();\n"
351351
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
352352

353353
+ " file.defaultValue = 'newDefault';\n"
354354
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
355355

356-
+ " document.forms[0].reset;\n"
356+
+ " document.forms[0].reset();\n"
357357
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
358358
+ " }\n"
359359
+ "</script>\n"
@@ -383,21 +383,21 @@ public void resetByJS() throws Exception {
383383
+ " var file = document.getElementById('testId');\n"
384384
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
385385

386-
+ " document.forms[0].reset;\n"
386+
+ " document.forms[0].reset();\n"
387387
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
388388

389389
+ " try{\n"
390390
+ " file.value = 'newValue';\n"
391391
+ " } catch(e) { logEx(e); }\n"
392392
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
393393

394-
+ " document.forms[0].reset;\n"
394+
+ " document.forms[0].reset();\n"
395395
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
396396

397397
+ " file.defaultValue = 'newDefault';\n"
398398
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
399399

400-
+ " document.forms[0].reset;\n"
400+
+ " document.forms[0].reset();\n"
401401
+ " log(file.value + '-' + file.defaultValue + '-' + file.getAttribute('value'));\n"
402402
+ " }\n"
403403
+ "</script>\n"
@@ -896,6 +896,20 @@ public void validationRequired() throws Exception {
896896
validation("<input type='file' id='e1' required>\n", "");
897897
}
898898

899+
/**
900+
* A DISABLED + required file input with no files selected must
901+
* still report itself as valid, since disabled fields are barred from
902+
* constraint validation entirely.
903+
* @throws Exception if an error occurs
904+
*/
905+
@Test
906+
@Alerts({"true",
907+
"false-false-false-false-false-false-false-false-false-false-true",
908+
"false"})
909+
public void validationRequiredDisabled() throws Exception {
910+
validation("<input type='file' id='e1' required disabled>\n", "");
911+
}
912+
899913
private void validation(final String htmlPart, final String jsPart) throws Exception {
900914
final String html = DOCTYPE_HTML
901915
+ "<html><head>\n"
@@ -1021,4 +1035,118 @@ public void valueWebkitdirectory() throws Exception {
10211035
driver.findElement(By.id("clickMe")).click();
10221036
verifyTitle2(driver, getExpectedAlerts());
10231037
}
1038+
1039+
/**
1040+
* Method reset() must not fire a change event, and must not try to
1041+
* resurrect a fake File from the 'value' attribute. This uses
1042+
* a REAL file selection (via sendKeys) so reset() has something genuine to
1043+
* clear, and counts onchange firings across the whole sequence.
1044+
* @throws Exception if the test fails
1045+
*/
1046+
@Test
1047+
@Alerts({"0-1"})
1048+
public void resetClearsRealFileSelectionWithoutSpuriousChange() throws Exception {
1049+
final String html = DOCTYPE_HTML
1050+
+ "<html><body>\n"
1051+
+ "<script>\n"
1052+
+ LOG_TITLE_FUNCTION
1053+
+ "var changeCount = 0;\n"
1054+
+ "</script>\n"
1055+
+ "<form id='form1'>\n"
1056+
+ " <input type='file' id='f' onchange='changeCount++;'>\n"
1057+
+ " <input type='reset' id='resetBtn'>\n"
1058+
+ "</form>\n"
1059+
+ "<button id='check' onclick='"
1060+
+ "log(document.getElementById(\"f\").files.length + \"-\" + changeCount);"
1061+
+ "'>check</button>\n"
1062+
+ "</body></html>";
1063+
1064+
final WebDriver driver = loadPage2(html);
1065+
final File tmpFile = File.createTempFile("htmlunit-test", ".txt");
1066+
try {
1067+
driver.findElement(By.id("f")).sendKeys(tmpFile.getAbsolutePath());
1068+
}
1069+
finally {
1070+
assertTrue(tmpFile.delete());
1071+
}
1072+
1073+
driver.findElement(By.id("resetBtn")).click();
1074+
driver.findElement(By.id("check")).click();
1075+
1076+
verifyTitle2(driver, getExpectedAlerts());
1077+
}
1078+
1079+
/**
1080+
* Calling reset() on a file input that was NEVER touched (no real selection made)
1081+
* must be a true no-op -- specifically must NOT fire a change event, since
1082+
* nothing actually changed.
1083+
* @throws Exception if the test fails
1084+
*/
1085+
@Test
1086+
@Alerts({"0"})
1087+
public void resetOnNeverTouchedFileInput_noChangeEventFired() throws Exception {
1088+
final String html = DOCTYPE_HTML
1089+
+ "<html><head>\n"
1090+
+ "<script>\n"
1091+
+ LOG_TITLE_FUNCTION
1092+
+ " function test() {\n"
1093+
+ " document.getElementById('resetBtn').click();\n"
1094+
+ " log(document.getElementById('f').files.length);\n"
1095+
+ " }\n"
1096+
+ "</script>\n"
1097+
+ "</head>\n"
1098+
+ "<body>\n"
1099+
+ "<form id='form1'>\n"
1100+
+ " <input type='file' id='f' onchange='log(\"unexpected change\");'>\n"
1101+
+ " <input type='reset' id='resetBtn'>\n"
1102+
+ "</form>\n"
1103+
+ "<button id='go' onclick='test()'>go</button>\n"
1104+
+ "</body></html>";
1105+
1106+
final WebDriver driver = loadPage2(html);
1107+
driver.findElement(By.id("go")).click();
1108+
1109+
verifyTitle2(DEFAULT_WAIT_TIME, driver, getExpectedAlerts());
1110+
}
1111+
1112+
/**
1113+
* Changing the 'type' attribute of an
1114+
* input away from 'file' and then back to 'file' should leave it with an
1115+
* empty value/file selection, not retain a stale selection from before the
1116+
* type change.
1117+
* @throws Exception if an error occurs
1118+
*/
1119+
@Test
1120+
@Alerts({"", "0"})
1121+
public void changingTypeAwayFromFileAndBackClearsSelection() throws Exception {
1122+
final String html = DOCTYPE_HTML
1123+
+ "<html><head>\n"
1124+
+ "<script>\n"
1125+
+ LOG_TITLE_FUNCTION
1126+
+ " function test() {\n"
1127+
+ " var input = document.getElementById('f');\n"
1128+
+ " input.type = 'text';\n"
1129+
+ " input.type = 'file';\n"
1130+
+ " log(input.value);\n"
1131+
+ " log(input.files.length);\n"
1132+
+ " }\n"
1133+
+ "</script>\n"
1134+
+ "</head>\n"
1135+
+ "<body>\n"
1136+
+ " <input type='file' id='f'>\n"
1137+
+ " <button id='go' onclick='test()'>go</button>\n"
1138+
+ "</body></html>";
1139+
1140+
final WebDriver driver = loadPage2(html);
1141+
final File tmpFile = File.createTempFile("htmlunit-test", ".txt");
1142+
try {
1143+
driver.findElement(By.id("f")).sendKeys(tmpFile.getAbsolutePath());
1144+
}
1145+
finally {
1146+
assertTrue(tmpFile.delete());
1147+
}
1148+
1149+
driver.findElement(By.id("go")).click();
1150+
verifyTitle2(driver, getExpectedAlerts()); // expect "" and "0"
1151+
}
10241152
}

0 commit comments

Comments
 (0)