Skip to content

Commit 2c625f3

Browse files
committed
Add NPE Guards
- Like values, names are only validated if they are not null Closes gh-9598
1 parent 6725b13 commit 2c625f3

File tree

2 files changed

+62
-8
lines changed

2 files changed

+62
-8
lines changed

Diff for: web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java

+19-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -610,19 +610,25 @@ private class StrictFirewalledRequest extends FirewalledRequest {
610610

611611
@Override
612612
public long getDateHeader(String name) {
613-
validateAllowedHeaderName(name);
613+
if (name != null) {
614+
validateAllowedHeaderName(name);
615+
}
614616
return super.getDateHeader(name);
615617
}
616618

617619
@Override
618620
public int getIntHeader(String name) {
619-
validateAllowedHeaderName(name);
621+
if (name != null) {
622+
validateAllowedHeaderName(name);
623+
}
620624
return super.getIntHeader(name);
621625
}
622626

623627
@Override
624628
public String getHeader(String name) {
625-
validateAllowedHeaderName(name);
629+
if (name != null) {
630+
validateAllowedHeaderName(name);
631+
}
626632
String value = super.getHeader(name);
627633
if (value != null) {
628634
validateAllowedHeaderValue(value);
@@ -632,7 +638,9 @@ public String getHeader(String name) {
632638

633639
@Override
634640
public Enumeration<String> getHeaders(String name) {
635-
validateAllowedHeaderName(name);
641+
if (name != null) {
642+
validateAllowedHeaderName(name);
643+
}
636644
Enumeration<String> headers = super.getHeaders(name);
637645
return new Enumeration<String>() {
638646

@@ -673,7 +681,9 @@ public String nextElement() {
673681

674682
@Override
675683
public String getParameter(String name) {
676-
validateAllowedParameterName(name);
684+
if (name != null) {
685+
validateAllowedParameterName(name);
686+
}
677687
String value = super.getParameter(name);
678688
if (value != null) {
679689
validateAllowedParameterValue(value);
@@ -717,7 +727,9 @@ public String nextElement() {
717727

718728
@Override
719729
public String[] getParameterValues(String name) {
720-
validateAllowedParameterName(name);
730+
if (name != null) {
731+
validateAllowedParameterName(name);
732+
}
721733
String[] values = super.getParameterValues(name);
722734
if (values != null) {
723735
for (String value : values) {

Diff for: web/src/test/java/org/springframework/security/web/firewall/StrictHttpFirewallTests.java

+43-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
2626
import org.springframework.http.HttpMethod;
2727
import org.springframework.mock.web.MockHttpServletRequest;
2828

29+
import static org.assertj.core.api.Assertions.assertThat;
2930
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
3031

3132
/**
@@ -690,4 +691,45 @@ public void getFirewalledRequestGetParameterValuesWhenNotAllowedInParameterNameT
690691
.isThrownBy(() -> request.getParameterValues("bad name"));
691692
}
692693

694+
// gh-9598
695+
@Test
696+
public void getFirewalledRequestGetParameterWhenNameIsNullThenIllegalArgumentException() {
697+
HttpServletRequest request = this.firewall.getFirewalledRequest(this.request);
698+
assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> request.getParameter(null));
699+
}
700+
701+
// gh-9598
702+
@Test
703+
public void getFirewalledRequestGetParameterValuesWhenNameIsNullThenIllegalArgumentException() {
704+
HttpServletRequest request = this.firewall.getFirewalledRequest(this.request);
705+
assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> request.getParameterValues(null));
706+
}
707+
708+
// gh-9598
709+
@Test
710+
public void getFirewalledRequestGetHeaderWhenNameIsNullThenNull() {
711+
HttpServletRequest request = this.firewall.getFirewalledRequest(this.request);
712+
assertThat(request.getHeader(null)).isNull();
713+
}
714+
715+
// gh-9598
716+
@Test
717+
public void getFirewalledRequestGetHeadersWhenNameIsNullThenEmptyEnumeration() {
718+
HttpServletRequest request = this.firewall.getFirewalledRequest(this.request);
719+
assertThat(request.getHeaders(null).hasMoreElements()).isFalse();
720+
}
721+
722+
// gh-9598
723+
@Test
724+
public void getFirewalledRequestGetIntHeaderWhenNameIsNullThenNegativeOne() {
725+
HttpServletRequest request = this.firewall.getFirewalledRequest(this.request);
726+
assertThat(request.getIntHeader(null)).isEqualTo(-1);
727+
}
728+
729+
@Test
730+
public void getFirewalledRequestGetDateHeaderWhenNameIsNullThenNegativeOne() {
731+
HttpServletRequest request = this.firewall.getFirewalledRequest(this.request);
732+
assertThat(request.getDateHeader(null)).isEqualTo(-1);
733+
}
734+
693735
}

0 commit comments

Comments
 (0)