Skip to content

Commit 42d7c55

Browse files
ecovaciecovaci
committed
Code refactoring: a quest for clean code
1 parent e7adf0f commit 42d7c55

File tree

8 files changed

+348
-251
lines changed

8 files changed

+348
-251
lines changed

‎src/main/java/org/kpax/winfoom/api/ApiController.java‎

Lines changed: 173 additions & 128 deletions
Large diffs are not rendered by default.

‎src/main/java/org/kpax/winfoom/proxy/ClientConnection.java‎

Lines changed: 47 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -320,42 +320,32 @@ void prepare() {
320320
}
321321
}
322322

323+
/**
324+
Prepare the request for execution:
325+
remove some headers, fix VIA header and set a proper entity.
326+
*/
323327
private void prepareRequest() {
324328
log.debug("Prepare the request for execution");
325-
// Prepare the request for execution:
326-
// remove some headers, fix VIA header and set a proper entity
327-
if (request instanceof HttpEntityEnclosingRequest) {
328-
log.debug("Set enclosing entity");
329-
RepeatableHttpEntity entity = new RepeatableHttpEntity(request,
330-
sessionInputBuffer,
331-
proxyConfig.getTempDirectory(),
332-
systemConfig.getInternalBufferLength());
333-
Header transferEncoding = request.getFirstHeader(HTTP.TRANSFER_ENCODING);
334-
if (transferEncoding != null
335-
&& StringUtils.containsIgnoreCase(transferEncoding.getValue(), HTTP.CHUNK_CODING)) {
336-
log.debug("Mark entity as chunked");
337-
entity.setChunked(true);
338-
339-
// Apache HttpClient adds a Transfer-Encoding header's chunk directive
340-
// so remove or strip the existent one from chunk directive
341-
request.removeHeader(transferEncoding);
342-
String nonChunkedTransferEncoding = HttpUtils.stripChunked(transferEncoding.getValue());
343-
if (StringUtils.isNotEmpty(nonChunkedTransferEncoding)) {
344-
request.addHeader(
345-
HttpUtils.createHttpHeader(HttpHeaders.TRANSFER_ENCODING,
346-
nonChunkedTransferEncoding));
347-
log.debug("Add chunk-striped request header");
348-
} else {
349-
log.debug("Remove transfer encoding chunked request header");
350-
}
351329

352-
}
353-
((HttpEntityEnclosingRequest) request).setEntity(entity);
330+
if (request instanceof HttpEntityEnclosingRequest) {
331+
prepareHttpEntityEnclosingRequest();
354332
} else {
355333
log.debug("No enclosing entity");
356334
}
357335

358-
// Remove banned headers
336+
removeBannedHeaders();
337+
fixViaHeader();
338+
}
339+
340+
private void fixViaHeader() {
341+
// Add a Via header and remove the existent one(s)
342+
Header viaHeader = request.getFirstHeader(HttpHeaders.VIA);
343+
request.removeHeaders(HttpHeaders.VIA);
344+
request.setHeader(HttpUtils.createViaHeader(request.getRequestLine().getProtocolVersion(),
345+
viaHeader));
346+
}
347+
348+
private void removeBannedHeaders() {
359349
List<String> bannedHeaders = request instanceof HttpEntityEnclosingRequest ?
360350
HttpUtils.ENTITY_BANNED_HEADERS : HttpUtils.DEFAULT_BANNED_HEADERS;
361351
for (Header header : request.getAllHeaders()) {
@@ -366,12 +356,35 @@ private void prepareRequest() {
366356
log.debug("Allow request header {}", header);
367357
}
368358
}
359+
}
369360

370-
// Add a Via header and remove the existent one(s)
371-
Header viaHeader = request.getFirstHeader(HttpHeaders.VIA);
372-
request.removeHeaders(HttpHeaders.VIA);
373-
request.setHeader(HttpUtils.createViaHeader(request.getRequestLine().getProtocolVersion(),
374-
viaHeader));
361+
private void prepareHttpEntityEnclosingRequest() {
362+
log.debug("Set enclosing entity");
363+
RepeatableHttpEntity entity = new RepeatableHttpEntity(request,
364+
sessionInputBuffer,
365+
proxyConfig.getTempDirectory(),
366+
systemConfig.getInternalBufferLength());
367+
Header transferEncoding = request.getFirstHeader(HTTP.TRANSFER_ENCODING);
368+
if (transferEncoding != null
369+
&& StringUtils.containsIgnoreCase(transferEncoding.getValue(), HTTP.CHUNK_CODING)) {
370+
log.debug("Mark entity as chunked");
371+
entity.setChunked(true);
372+
373+
// Apache HttpClient adds a Transfer-Encoding header's chunk directive
374+
// so remove or strip the existent one from chunk directive
375+
request.removeHeader(transferEncoding);
376+
String nonChunkedTransferEncoding = HttpUtils.stripChunked(transferEncoding.getValue());
377+
if (StringUtils.isNotEmpty(nonChunkedTransferEncoding)) {
378+
request.addHeader(
379+
HttpUtils.createHttpHeader(HttpHeaders.TRANSFER_ENCODING,
380+
nonChunkedTransferEncoding));
381+
log.debug("Add chunk-striped request header");
382+
} else {
383+
log.debug("Remove transfer encoding chunked request header");
384+
}
385+
386+
}
387+
((HttpEntityEnclosingRequest) request).setEntity(entity);
375388
}
376389

377390
@Override

‎src/main/java/org/kpax/winfoom/proxy/LocalProxyServer.java‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,7 @@ synchronized void start() throws IOException {
7373
executorService.submit(() -> {
7474
while (true) {
7575
try {
76-
Socket socket = serverSocket.accept();
77-
systemConfig.configureSocket(socket);
78-
executorService.submit(() -> {
79-
try {
80-
clientConnectionHandler.handleConnection(socket);
81-
} catch (Exception e) {
82-
log.debug("Error on handling connection", e);
83-
} finally {
84-
InputOutputs.close(socket);
85-
}
86-
});
76+
initiateSocketConnection(clientConnectionHandler);
8777
} catch (SocketException e) {
8878

8979
// The ServerSocket has been closed, exit the while loop
@@ -109,6 +99,20 @@ synchronized void start() throws IOException {
10999
}
110100
}
111101

102+
private void initiateSocketConnection(ClientConnectionHandler clientConnectionHandler) throws IOException {
103+
Socket socket = serverSocket.accept();
104+
systemConfig.configureSocket(socket);
105+
executorService.submit(() -> {
106+
try {
107+
clientConnectionHandler.handleConnection(socket);
108+
} catch (Exception e) {
109+
log.debug("Error on handling connection", e);
110+
} finally {
111+
InputOutputs.close(socket);
112+
}
113+
});
114+
}
115+
112116
@Override
113117
public synchronized void onStop() {
114118
log.info("Close the local proxy server");

‎src/main/java/org/kpax/winfoom/proxy/PacClientConnectionHandler.java‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,20 +55,17 @@ void processConnection(ClientConnection clientConnection) throws Exception {
5555
URI requestUri = clientConnection.getRequestUri();
5656
log.debug("Extracted URI from request {}", requestUri);
5757

58-
List<ProxyInfo> activeProxies;
59-
try {
60-
activeProxies = pacScriptEvaluator.findProxyForURL(requestUri);
61-
log.debug("activeProxies: {}", activeProxies);
62-
} catch (Exception e) {
63-
clientConnection.writeErrorResponse(HttpStatus.SC_INTERNAL_SERVER_ERROR, HttpUtils.reasonPhraseForPac(e));
64-
throw e;
65-
}
58+
List<ProxyInfo> activeProxies = getActiveProxies(clientConnection, requestUri);
6659

6760
if (activeProxies.isEmpty()) {
6861
clientConnection.writeBadGatewayResponse("Proxy Auto Config error: no available proxy server");
6962
throw new IllegalStateException("All proxy servers are blacklisted!");
7063
}
7164

65+
processClientConnection(clientConnection, activeProxies);
66+
}
67+
68+
private void processClientConnection(ClientConnection clientConnection, List<ProxyInfo> activeProxies) {
7269
for (Iterator<ProxyInfo> itr = activeProxies.iterator(); itr.hasNext(); ) {
7370
ProxyInfo proxy = itr.next();
7471
ClientConnectionProcessor connectionProcessor = connectionProcessorSelector.select(clientConnection.isConnect(),
@@ -90,4 +87,16 @@ void processConnection(ClientConnection clientConnection) throws Exception {
9087
}
9188
}
9289
}
90+
91+
private List<ProxyInfo> getActiveProxies(ClientConnection clientConnection, URI requestUri) throws Exception {
92+
List<ProxyInfo> activeProxies;
93+
try {
94+
activeProxies = pacScriptEvaluator.findProxyForURL(requestUri);
95+
log.debug("activeProxies: {}", activeProxies);
96+
} catch (Exception e) {
97+
clientConnection.writeErrorResponse(HttpStatus.SC_INTERNAL_SERVER_ERROR, HttpUtils.reasonPhraseForPac(e));
98+
throw e;
99+
}
100+
return activeProxies;
101+
}
93102
}

‎src/main/java/org/kpax/winfoom/proxy/ProxyController.java‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,21 +67,14 @@ public synchronized void start() throws Exception {
6767
sorted(AnnotationAwareOrderComparator.INSTANCE).
6868
map(b -> (StartListener) b).
6969
collect(Collectors.toList());
70+
7071
try {
71-
for (StartListener startListener : startListeners) {
72-
TypeQualifier typeQualifier = startListener.getClass().getMethod("onStart").
73-
getDeclaredAnnotation(TypeQualifier.class);
74-
if (typeQualifier == null || typeQualifier.value() == proxyConfig.getProxyType()) {
75-
log.debug("Call onStart for: {}", startListener.getClass());
76-
startListener.onStart();
77-
} else {
78-
log.debug("onStart ignored for {}", startListener.getClass());
79-
}
80-
}
72+
executeOnStartListeners(startListeners);
8173
} catch (Exception e) {
8274
resetState();
8375
throw e;
8476
}
77+
8578
if (proxyConfig.getProxyType().isSocks5() || proxyConfig.isPacAuthManualMode()) {
8679
Authenticator.setDefault(new Authenticator() {
8780
public PasswordAuthentication getPasswordAuthentication() {
@@ -95,6 +88,19 @@ public PasswordAuthentication getPasswordAuthentication() {
9588
started = true;
9689
}
9790

91+
private void executeOnStartListeners(List<StartListener> startListeners) throws Exception {
92+
for (StartListener startListener : startListeners) {
93+
TypeQualifier typeQualifier = startListener.getClass().getMethod("onStart").
94+
getDeclaredAnnotation(TypeQualifier.class);
95+
if (typeQualifier == null || typeQualifier.value() == proxyConfig.getProxyType()) {
96+
log.debug("Call onStart for: {}", startListener.getClass());
97+
startListener.onStart();
98+
} else {
99+
log.debug("onStart ignored for {}", startListener.getClass());
100+
}
101+
}
102+
}
103+
98104
/**
99105
* End the proxy session.
100106
*/

‎src/main/java/org/kpax/winfoom/proxy/ProxyExecutorService.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ public <T> T invokeAny(Collection<? extends Callable<T>> tasks, long timeout, Ti
9494
return threadPoolSupplier.get().invokeAny(tasks, timeout, unit);
9595
}
9696

97+
@Override
98+
public void close() {
99+
StopListener.super.close();
100+
}
101+
97102
public boolean isShutdown() {
98103
return threadPoolSupplier.hasValue() && threadPoolSupplier.get().isShutdown();
99104
}
@@ -115,9 +120,7 @@ public static class DefaultThreadFactory implements ThreadFactory {
115120
private final String namePrefix;
116121

117122
public DefaultThreadFactory() {
118-
SecurityManager securityManager = System.getSecurityManager();
119-
group = (securityManager != null) ? securityManager.getThreadGroup() :
120-
Thread.currentThread().getThreadGroup();
123+
group = Thread.currentThread().getThreadGroup();
121124
namePrefix = "pool-" +
122125
poolNumber.getAndIncrement() +
123126
"-thread-";

‎src/main/java/org/kpax/winfoom/proxy/RepeatableHttpEntity.java‎

Lines changed: 49 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -173,50 +173,60 @@ private void writeStream(OutputStream outStream) throws IOException {
173173
try (CacheFile cacheFile = CacheFile.from(tempFilepath, buffer)) {
174174
if (contentLength < 0) {
175175
if (isChunked()) {
176-
ChunkedInputStream chunkedInputStream = new ChunkedInputStream(inputBuffer);
177-
int length;
178-
while ((length = chunkedInputStream.read(buffer)) > 0) {
179-
outStream.write(buffer, 0, length);
180-
outStream.flush();
181-
182-
// Write to file
183-
cacheFile.write(length);
184-
}
176+
writeChunkedStream(outStream, buffer, cacheFile);
185177
} else {
186-
187-
// consume until EOF
188-
int length;
189-
while (InputOutputs.isAvailable(inputBuffer)) {
190-
length = inputBuffer.read(buffer);
191-
if (length == -1) {
192-
break;
193-
}
194-
outStream.write(buffer, 0, length);
195-
outStream.flush();
196-
197-
// Write to file
198-
cacheFile.write(length);
199-
}
178+
writeNonChunkedStream(outStream, buffer, cacheFile);
200179
}
201-
202180
} else {
203-
long remaining = contentLength;
204-
205-
// consume no more than maxLength
206-
int length;
207-
while (remaining > 0 && InputOutputs.isAvailable(inputBuffer)) {
208-
length = inputBuffer.read(buffer, 0, (int) Math.min(OUTPUT_BUFFER_SIZE, remaining));
209-
if (length == -1) {
210-
break;
211-
}
212-
outStream.write(buffer, 0, length);
213-
outStream.flush();
214-
remaining -= length;
181+
writeStreamWithContent(outStream, buffer, cacheFile);
182+
}
183+
}
184+
}
215185

216-
// Write to temp file
217-
cacheFile.write(length);
218-
}
186+
private void writeStreamWithContent(OutputStream outStream, byte[] buffer, CacheFile cacheFile) throws IOException {
187+
long remaining = contentLength;
188+
189+
// consume no more than maxLength
190+
int length;
191+
while (remaining > 0 && InputOutputs.isAvailable(inputBuffer)) {
192+
length = inputBuffer.read(buffer, 0, (int) Math.min(OUTPUT_BUFFER_SIZE, remaining));
193+
if (length == -1) {
194+
break;
195+
}
196+
outStream.write(buffer, 0, length);
197+
outStream.flush();
198+
remaining -= length;
199+
200+
// Write to temp file
201+
cacheFile.write(length);
202+
}
203+
}
204+
205+
private void writeNonChunkedStream(OutputStream outStream, byte[] buffer, CacheFile cacheFile) throws IOException {
206+
// consume until EOF
207+
int length;
208+
while (InputOutputs.isAvailable(inputBuffer)) {
209+
length = inputBuffer.read(buffer);
210+
if (length == -1) {
211+
break;
219212
}
213+
outStream.write(buffer, 0, length);
214+
outStream.flush();
215+
216+
// Write to file
217+
cacheFile.write(length);
218+
}
219+
}
220+
221+
private void writeChunkedStream(OutputStream outStream, byte[] buffer, CacheFile cacheFile) throws IOException {
222+
ChunkedInputStream chunkedInputStream = new ChunkedInputStream(inputBuffer);
223+
int length;
224+
while ((length = chunkedInputStream.read(buffer)) > 0) {
225+
outStream.write(buffer, 0, length);
226+
outStream.flush();
227+
228+
// Write to file
229+
cacheFile.write(length);
220230
}
221231
}
222232

0 commit comments

Comments
 (0)