Skip to content

Commit

Permalink
Fix 165 (#166)
Browse files Browse the repository at this point in the history
* Allows only one form of rate limiting per request

* Added the breakOnMatch option so that the previous commit can either be enabled or disabled for every policy

* Added CIDR block capabilities.

* Changed the method used to deal with the IP addresses by adding SubnetUtils class.

* Fixed minor formatting issues.

* Added try catch block so that it won't throw errors when given incorrect input, it will just return false.

* Changed try catch block to catch exceptions instead of errors.

* Switched from using a class for SubnetUtils to an imported library and made changes to the unit tests.

* Fixed test case header assertions.
  • Loading branch information
andrew-holman authored and marcosbarbero committed Jul 8, 2019
1 parent e9db1a9 commit 682cfcc
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 5 deletions.
5 changes: 5 additions & 0 deletions spring-cloud-zuul-ratelimit-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@
<artifactId>spring-security-test</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>commons-net</groupId>
<artifactId>commons-net</artifactId>
<version>3.6</version>
</dependency>

</dependencies>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.RateLimitUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.net.util.SubnetUtils;
import org.springframework.cloud.netflix.zuul.filters.Route;

import javax.servlet.http.HttpServletRequest;
Expand All @@ -30,6 +31,15 @@ public enum RateLimitType {
ORIGIN {
@Override
public boolean apply(HttpServletRequest request, Route route, RateLimitUtils rateLimitUtils, String matcher) {
if(matcher.contains("/")) {
try {
SubnetUtils subnetUtils = new SubnetUtils(matcher);
return subnetUtils.getInfo().isInRange(rateLimitUtils.getRemoteAddress(request));
}
catch(Exception e) {
return false;
}
}
return matcher.equals(rateLimitUtils.getRemoteAddress(request));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,15 @@ public ResponseEntity<String> serviceF() {
public ResponseEntity<String> serviceG() {
return ResponseEntity.ok(RESPONSE_BODY);
}

@GetMapping("/serviceH")
public ResponseEntity<String> serviceH() {
return ResponseEntity.ok(RESPONSE_BODY);
}

@GetMapping("/serviceI")
public ResponseEntity<String> serviceI() {
return ResponseEntity.ok(RESPONSE_BODY);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ zuul:
serviceG:
path: /serviceG
url: forward:/
serviceH:
path: /serviceH
url: forward:/
serviceI:
path: /serviceI
url: forward:/
ratelimit:
enabled: true
repository: JPA
Expand Down Expand Up @@ -80,6 +86,7 @@ zuul:
refresh-interval: 60
type:
- origin
breakOnMatch: true
serviceG:
- limit: 2
refresh-interval: 60
Expand All @@ -90,6 +97,29 @@ zuul:
refresh-interval: 60
type:
- origin
breakOnMatch: true
serviceH:
- limit: 2
refresh-interval: 60
type:
- origin=127.0.0.0/22
breakOnMatch: true
- limit: 1
refresh-interval: 60
type:
- origin
breakOnMatch: true
serviceI:
- limit: 2
refresh-interval: 60
type:
- origin=126.0.0.0/22
breakOnMatch: true
- limit: 1
refresh-interval: 60
type:
- origin
breakOnMatch: true
strip-prefix: true

logging:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,13 @@ public void testExceedingQuotaCapacityRequest() {
public void testUsingBreakOnMatchSpecificCase() {
ResponseEntity<String> response = this.restTemplate.getForEntity("/serviceF", String.class);
HttpHeaders headers = response.getHeaders();
String key = "rate-limit-application_serviceF_127.0.0.1";
assertHeaders(headers, key, true, false);
String key = "rate-limit-application_serviceF_127.0.0.1_127.0.0.1";
assertHeaders(headers, key, false, false);
assertEquals(OK, response.getStatusCode());

response = this.restTemplate.getForEntity("/serviceF", String.class);
headers = response.getHeaders();
assertHeaders(headers, key, true, false);
assertHeaders(headers, key, false, false);
assertEquals(OK, response.getStatusCode());
}

Expand All @@ -144,13 +144,27 @@ public void testUsingBreakOnMatchGeneralCase() {
ResponseEntity<String> response = this.restTemplate.getForEntity("/serviceG", String.class);
HttpHeaders headers = response.getHeaders();
String key = "rate-limit-application_serviceG_127.0.0.1";
assertHeaders(headers, key, true, false);
assertHeaders(headers, key, false, false);
assertEquals(OK, response.getStatusCode());

response = this.restTemplate.getForEntity("/serviceG", String.class);
headers = response.getHeaders();
assertHeaders(headers, key, true, false);
assertHeaders(headers, key, false, false);
assertEquals(TOO_MANY_REQUESTS, response.getStatusCode());
}

@Test
public void testUsingBreakOnMatchGeneralCaseWithCidr() {
ResponseEntity<String> response = this.restTemplate.getForEntity("/serviceI", String.class);
HttpHeaders headers = response.getHeaders();
String key = "rate-limit-application_serviceI_127.0.0.1";
assertHeaders(headers, key, false, false);
assertEquals(OK, response.getStatusCode());

response = this.restTemplate.getForEntity("/serviceI", String.class);
headers = response.getHeaders();
assertHeaders(headers, key, false, false);
assertEquals(TOO_MANY_REQUESTS, response.getStatusCode());
}

private void assertHeaders(HttpHeaders headers, String key, boolean nullable, boolean quotaHeaders) {
Expand Down

0 comments on commit 682cfcc

Please sign in to comment.