Skip to content

Commit

Permalink
Temptative fix
Browse files Browse the repository at this point in the history
  • Loading branch information
enricovianello committed Dec 10, 2024
1 parent c9f19ae commit 0af1bd3
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 63 deletions.
12 changes: 12 additions & 0 deletions robot/test/tpc.robot
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,18 @@ Pull copy works oauth and https
Curl Success ${dest} ${davix.opts.oauth}
[Teardown] Pull copy works oauth and https Teardown

Pull copy works oauth and https with Expect 100-Continue
[Tags] oauth tpc
[Setup] Pull copy works oauth and https Setup
${dest} DAVS URL tpc_test_oauth_https
${src} Remote DAVS URL tpc_test_oauth_https sa=${sa.oauth}
${opts} Set Variable -H "TransferHeaderAuthorization: Bearer %{${cred.oauth.env_var_name}}" ${curl.opts.default}
${opts} Set Variable -H "ClientInfo: job-id=123; file-id=454; retry=0" ${opts}
${opts} Set Variable -H "Expect: 100-Continue" ${opts}
${rc} ${out} Curl Voms Pull COPY Success ${dest} ${src} ${opts}
Should Contain ${out} 100 Continue
[Teardown] Pull copy works oauth and https Teardown

Push copy works
[Tags] voms oauth tpc push
[Setup] Push copy works Setup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ public class ThirdPartyCopyProperties {
@Positive(message = "tpc.progressReportThreadPoolSize must be a positive integer (i.e. > 0)")
int progressReportThreadPoolSize;

@Min(value = 0, message = "Threshold of content size to reach before sending 'Expect: 100-continue' header.")
long enableExpectContinueThreshold;

public String getTlsProtocol() {
return tlsProtocol;
}
Expand Down Expand Up @@ -138,12 +135,4 @@ public void setProgressReportThreadPoolSize(int progressReportThreadPoolSize) {
this.progressReportThreadPoolSize = progressReportThreadPoolSize;
}

public long getEnableExpectContinueThreshold() {
return enableExpectContinueThreshold;
}

public void setEnableExpectContinueThreshold(long enableExpectContinueThreshold) {
this.enableExpectContinueThreshold = enableExpectContinueThreshold;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ CloseableHttpClient transferClient(ThirdPartyCopyProperties props,

int timeoutMsec = (int) TimeUnit.SECONDS.toMillis(props.getTimeoutInSecs());
RequestConfig config = RequestConfig.custom()
.setExpectContinueEnabled(false)
.setExpectContinueEnabled(true)
.setConnectTimeout(timeoutMsec)
.setConnectionRequestTimeout(timeoutMsec)
.setSocketTimeout(timeoutMsec)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ FilterRegistrationBean<TransferFilter> tpcFilter(Clock clock, FilesystemAccess f

FilterRegistrationBean<TransferFilter> tpcFilter =
new FilterRegistrationBean<>(new TransferFilter(clock, metricsClient, resolver, lus,
props.isVerifyChecksum(), props.getEnableExpectContinueThreshold()));
props.isVerifyChecksum()));
tpcFilter.addUrlPatterns("/*");
tpcFilter.setOrder(TPC_FILTER_ORDER);
return tpcFilter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public class TransferFilter extends TransferFilterSupport implements Filter {
final TransferClient client;

public TransferFilter(Clock clock, TransferClient c, PathResolver resolver, LocalURLService lus,
boolean verifyChecksum, long enableExpectContinueThreshold) {
super(clock, resolver, lus, verifyChecksum, enableExpectContinueThreshold);
boolean verifyChecksum) {
super(clock, resolver, lus, verifyChecksum);
client = c;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,14 @@ public class TransferFilterSupport implements TpcUtils {
protected final LocalURLService localURLService;
protected final boolean verifyChecksum;
protected final TransferStatus.Builder status;
protected long enableExpectContinueThreshold;


protected TransferFilterSupport(Clock clock, PathResolver resolver, LocalURLService lus,
boolean verifyChecksum, long enableExpectContinueThreshold) {
boolean verifyChecksum) {
this.clock = clock;
this.resolver = resolver;
this.localURLService = lus;
this.verifyChecksum = verifyChecksum;
this.enableExpectContinueThreshold = enableExpectContinueThreshold;
status = TransferStatus.builder(clock);
}

Expand Down Expand Up @@ -99,11 +97,6 @@ protected Multimap<String, String> getTransferHeaders(HttpServletRequest request
}
}

if (isPushTpc(request, localURLService) && request.getContentLength() >= enableExpectContinueThreshold) {
xferHeaders.put(org.apache.http.protocol.HTTP.EXPECT_DIRECTIVE,
org.apache.http.protocol.HTTP.EXPECT_CONTINUE);
}

return xferHeaders;
}

Expand Down
1 change: 0 additions & 1 deletion src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ tpc:
use-conscrypt: ${STORM_WEBDAV_TPC_USE_CONSCRYPT:false}
enable-tls-client-auth: ${STORM_WEBDAV_TPC_ENABLE_TLS_CLIENT_AUTH:false}
progress-report-thread-pool-size: ${STORM_WEBDAV_TPC_PROGRESS_REPORT_THREAD_POOL_SIZE:4}
enable-expect-continue-threshold: ${STORM_WEBDAV_TPC_ENABLE_EXPECT_CONTINUE_THRESHOLD:1048576}

oauth:
refresh-period-minutes: ${STORM_WEBDAV_OAUTH_REFRESH_PERIOD_MINUTES:60}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,44 +173,6 @@ void unresolvedSourcePathFailsRequest() throws IOException, ServletException {

}

@Test
void checkExpectContinueHeaderIsSet() throws IOException, ServletException {

when(request.getHeader(TRANSFER_HEADER_AUTHORIZATION_KEY))
.thenReturn(TRANSFER_HEADER_AUTHORIZATION_VALUE);
when(request.getHeaderNames()).thenReturn(
enumeration(asList(TRANSFER_HEADER_AUTHORIZATION_KEY)));
when(request.getContentLength()).thenReturn(1024*1024+1);

filter.doFilter(request, response, chain);
verify(client).handle(putXferRequest.capture(), Mockito.any());

Multimap<String, String> xferHeaders = putXferRequest.getValue().transferHeaders();
assertThat(xferHeaders.size(), is(2));

assertThat(xferHeaders.containsKey(EXPECTED_HEADER), is(true));
assertThat(xferHeaders.get(EXPECTED_HEADER).iterator().next(), is(EXPECTED_VALUE));

}

@Test
void checkExpectContinueHeaderIsNotSet() throws IOException, ServletException {

when(request.getHeader(TRANSFER_HEADER_AUTHORIZATION_KEY))
.thenReturn(TRANSFER_HEADER_AUTHORIZATION_VALUE);
when(request.getHeaderNames()).thenReturn(
enumeration(asList(TRANSFER_HEADER_AUTHORIZATION_KEY)));
when(request.getContentLength()).thenReturn(1024*1024-1);

filter.doFilter(request, response, chain);
verify(client).handle(putXferRequest.capture(), Mockito.any());

Multimap<String, String> xferHeaders = putXferRequest.getValue().transferHeaders();
assertThat(xferHeaders.size(), is(1));

assertThat(xferHeaders.containsKey(EXPECTED_HEADER), is(false));
}

@Test
void bothSciTagAndTransferHeaderSciTag() throws IOException, ServletException {
when(request.getHeaderNames())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public class TransferFilterTestSupport {
LocalURLService lus = new StaticHostListLocalURLService(Arrays.asList("localhost"));

protected void setup() throws IOException {
filter = new TransferFilter(clock, client, resolver, lus, true, 1024L*1024L);
filter = new TransferFilter(clock, client, resolver, lus, true);
lenient().when(request.getHeaderNames()).thenReturn(requestHeaderNames);

}
Expand Down

0 comments on commit 0af1bd3

Please sign in to comment.