Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prototype of rewrite of CLI to not use Tyrus #9688

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

Prototype of rewrite of CLI to not use Tyrus #9688

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
856da1d
Prototype of rewrite of `CLI` to not use Tyrus
jglick Sep 4, 2024
bdb0e98
Merge branch 'master' of https://github.com/jenkinsci/jenkins into CL…
jglick Nov 8, 2024
ebf9b8b
`Origin` header mandatory after de450967f38398169650b55c002f1229a3fcdb1b
jglick Nov 8, 2024
4084c62
`last=true` https://github.com/jenkinsci/jenkins/pull/9688#discussion…
jglick Nov 8, 2024
08e9e3c
For whatever reason, `onBinary` is never called, so the client just h…
jglick Nov 8, 2024
ba9591d
https://docs.oracle.com/en/java/javase/17/docs/api/java.net.http/java…
jglick Nov 8, 2024
88fadbf
Clearer assertion
jglick Nov 12, 2024
0bd112c
Update CLIActionTest.java
timja Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions cli/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,6 @@
<version>${mina-sshd.version}</version>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.glassfish.tyrus.bundles</groupId>
<artifactId>tyrus-standalone-client-jdk</artifactId>
<version>2.2.0</version>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.jenkins-ci</groupId>
<artifactId>annotation-indexer</artifactId>
Expand Down Expand Up @@ -141,10 +135,6 @@
<pattern>com</pattern>
<shadedPattern>io.jenkins.cli.shaded.com</shadedPattern>
</relocation>
<relocation>
<pattern>jakarta</pattern>
<shadedPattern>io.jenkins.cli.shaded.jakarta</shadedPattern>
</relocation>
</relocations>
<transformers>
<transformer implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
Expand Down
140 changes: 83 additions & 57 deletions cli/src/main/java/hudson/cli/CLI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.cli.client.Messages;
import jakarta.websocket.ClientEndpointConfig;
import jakarta.websocket.Endpoint;
import jakarta.websocket.EndpointConfig;
import jakarta.websocket.HandshakeResponse;
import jakarta.websocket.Session;
import java.io.DataInputStream;
import java.io.File;
import java.io.IOException;
Expand All @@ -42,6 +37,9 @@
import java.net.URI;
import java.net.URL;
import java.net.URLConnection;
import java.net.http.HttpClient;
import java.net.http.WebSocket;
import java.net.http.WebSocketHandshakeException;
import java.nio.ByteBuffer;
import java.nio.charset.Charset;
import java.nio.file.Files;
Expand All @@ -54,19 +52,16 @@
import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.CompletionStage;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import org.glassfish.tyrus.client.ClientManager;
import org.glassfish.tyrus.client.ClientProperties;
import org.glassfish.tyrus.client.SslEngineConfigurator;
import org.glassfish.tyrus.client.exception.DeploymentHandshakeException;
import org.glassfish.tyrus.container.jdk.client.JdkClientContainer;

/**
* CLI entry point to Jenkins.
Expand Down Expand Up @@ -336,71 +331,102 @@ private static File getFileFromArguments(List<String> args) {

private static int webSocketConnection(String url, List<String> args, CLIConnectionFactory factory) throws Exception {
LOGGER.fine(() -> "Trying to connect to " + url + " via plain protocol over WebSocket");
class CLIEndpoint extends Endpoint {
@Override
public void onOpen(Session session, EndpointConfig config) {}
}

class Authenticator extends ClientEndpointConfig.Configurator {
HandshakeResponse hr;
@Override
public void beforeRequest(Map<String, List<String>> headers) {
if (factory.authorization != null) {
headers.put("Authorization", List.of(factory.authorization));
}
}
@Override
public void afterResponse(HandshakeResponse hr) {
this.hr = hr;
}
var wsb = HttpClient.newBuilder().followRedirects(HttpClient.Redirect.NORMAL).build().newWebSocketBuilder();
if (factory.authorization != null) {
wsb.header("Authorization", factory.authorization);
}
var authenticator = new Authenticator();

ClientManager client = ClientManager.createClient(JdkClientContainer.class.getName()); // ~ ContainerProvider.getWebSocketContainer()
client.getProperties().put(ClientProperties.REDIRECT_ENABLED, true); // https://tyrus-project.github.io/documentation/1.13.1/index/tyrus-proprietary-config.html#d0e1775
var controllerUri = new URI(url);
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
wsb.header("Origin", new URI(controllerUri.getScheme(), null, controllerUri.getHost(), controllerUri.getPort(), null, null, null).toString());
if (factory.noCertificateCheck) {
SSLContext sslContext = SSLContext.getInstance("TLS");
sslContext.init(null, new TrustManager[] {new NoCheckTrustManager()}, new SecureRandom());
/* TODO
SslEngineConfigurator sslEngineConfigurator = new SslEngineConfigurator(sslContext);
sslEngineConfigurator.setHostnameVerifier((s, sslSession) -> true);
client.getProperties().put(ClientProperties.SSL_ENGINE_CONFIGURATOR, sslEngineConfigurator);
*/
}
Session session;
try {
session = client.connectToServer(new CLIEndpoint(), ClientEndpointConfig.Builder.create().configurator(authenticator).build(), URI.create(url.replaceFirst("^http", "ws") + "cli/ws"));
} catch (DeploymentHandshakeException x) {
System.err.println("CLI handshake failed with status code " + x.getHttpStatusCode());
if (authenticator.hr != null) {
for (var entry : authenticator.hr.getHeaders().entrySet()) {
// org.glassfish.tyrus.core.Utils.parseHeaderValue improperly splits values like Date at commas, so undo that:
System.err.println(entry.getKey() + ": " + String.join(", ", entry.getValue()));
}
// UpgradeResponse.getReasonPhrase is useless since Jetty generates it from the code,
// and the body is not accessible at all.
}
return 15; // compare CLICommand.main
}
PlainCLIProtocol.Output out = new PlainCLIProtocol.Output() {
var ws = new AtomicReference<WebSocket>();
var out = new PlainCLIProtocol.Output() {
@Override
public void send(byte[] data) throws IOException {
session.getBasicRemote().sendBinary(ByteBuffer.wrap(data));
ws.get().sendBinary(ByteBuffer.wrap(data), true);
}

@Override
public void close() throws IOException {
session.close();
var _ws = ws.get();
if (_ws != null) {
_ws.sendClose(WebSocket.NORMAL_CLOSURE, "");
}
}
};
try (ClientSideImpl connection = new ClientSideImpl(out)) {
session.addMessageHandler(InputStream.class, is -> {
try {
connection.handle(new DataInputStream(is));
} catch (IOException x) {
LOGGER.log(Level.WARNING, null, x);
ws.set(wsb.buildAsync(URI.create(url.replaceFirst("^http", "ws") + "cli/ws"), new WebSocket.Listener() {
@Override
public CompletionStage<?> onBinary(WebSocket webSocket, ByteBuffer data, boolean last) {
// TODO if !last, buffer up (though CLIAction.ws does not currently send partial messages)
try {
connection.handle(new DataInputStream(new ByteBufferBackedInputStream(data)));
} catch (IOException x) {
x.printStackTrace();
LOGGER.log(Level.WARNING, null, x);
}
webSocket.request(1);
return null;
}
@Override
public void onOpen(WebSocket webSocket) {
webSocket.request(1);
}
@Override
public void onError(WebSocket webSocket, Throwable error) {
LOGGER.log(Level.WARNING, null, error);
}
});
}).get());
connection.start(args);
return connection.exit();
} catch (ExecutionException x) {
var cause = x.getCause();
if (cause instanceof WebSocketHandshakeException) {
var rsp = ((WebSocketHandshakeException) cause).getResponse();
System.err.println("CLI handshake failed with status code " + rsp.statusCode());
for (var entry : rsp.headers().map().entrySet()) {
for (var value : entry.getValue()) {
System.err.println(entry.getKey() + ": " + value);
}
}
// WebSocketHandshakeException.getResponse is available but HttpResponse.body is empty
return 15; // compare CLICommand.main
} else if (cause instanceof Exception x2) {
throw x2;
} else {
throw new Exception(cause);
}
}
}

// https://stackoverflow.com/a/6603018/12916
private static final class ByteBufferBackedInputStream extends InputStream {
final ByteBuffer buf;
ByteBufferBackedInputStream(ByteBuffer buf) {
this.buf = buf;
}
@Override
public int read() throws IOException {
if (!buf.hasRemaining()) {
return -1;
}
return buf.get() & 0xFF;
}
@Override
public int read(byte[] bytes, int off, int len) throws IOException {
if (!buf.hasRemaining()) {
return -1;
}
len = Math.min(len, buf.remaining());
buf.get(bytes, off, len);
return len;
}
}

Expand Down
1 change: 1 addition & 0 deletions core/src/main/java/hudson/cli/CLIAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public HttpResponse doWs(StaplerRequest2 req) {
LOGGER.log(Level.FINE, () -> "Rejecting origin: " + actualOrigin + "; expected was from request: " + expectedOrigin);
return HttpResponses.forbidden();
}
LOGGER.fine(() -> "accepting Origin: " + actualOrigin);
} else if (!ALLOW_WEBSOCKET) {
return HttpResponses.forbidden();
}
Expand Down
8 changes: 4 additions & 4 deletions test/src/test/java/hudson/cli/CLIActionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import org.apache.commons.io.input.NullInputStream;
import org.apache.commons.io.output.CountingOutputStream;
import org.apache.commons.io.output.TeeOutputStream;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
Expand Down Expand Up @@ -136,7 +135,7 @@ private void assertExitCode(int code, boolean useApiToken, File jar, String... a
assertEquals(code, proc.join());
}

@Ignore("TODO flaky test") @Test public void authenticationFailed() throws Exception {
@Test public void authenticationFailed() throws Exception {
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.ADMINISTER).everywhere().toAuthenticated());
var jar = tmp.newFile("jenkins-cli.jar");
Expand All @@ -145,20 +144,21 @@ private void assertExitCode(int code, boolean useApiToken, File jar, String... a
var exitStatus = new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(
"java", "-jar", jar.getAbsolutePath(), "-s", j.getURL().toString(), "-auth", "user:bogustoken", "who-am-i"
).stdout(baos).start().join();
assertThat(baos.toString(), allOf(containsString("status code 401"), containsString("Server: Jetty")));
assertThat(baos.toString(), allOf(containsString("status code 401"), containsString("erver: Jetty")));
jglick marked this conversation as resolved.
Show resolved Hide resolved
assertThat(exitStatus, is(15));
}

@Issue("JENKINS-41745")
@Test
public void encodingAndLocale() throws Exception {
logging.record(CLIAction.class, Level.FINE);
File jar = tmp.newFile("jenkins-cli.jar");
FileUtils.copyURLToFile(j.jenkins.getJnlpJars("jenkins-cli.jar").getURL(), jar);
ByteArrayOutputStream baos = new ByteArrayOutputStream();
assertEquals(0, new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(
"java", "-Dfile.encoding=ISO-8859-2", "-Duser.language=cs", "-Duser.country=CZ", "-jar", jar.getAbsolutePath(),
"-s", j.getURL().toString()./* just checking */replaceFirst("/$", ""), "test-diagnostic").
stdout(baos).stderr(System.err).join());
stdout(new TeeOutputStream(baos, System.out)).stderr(System.err).join());
assertEquals("encoding=ISO-8859-2 locale=cs_CZ", baos.toString(Charset.forName("ISO-8859-2")).trim());
// TODO test that stdout/stderr are in expected encoding (not true of -remoting mode!)
// -ssh mode does not pass client locale or encoding
Expand Down
Loading