Fixing issue where WILD XAuthority entries do not have an address. Fi…

…xed major bug when writing requests where the header for extensions did not replace the second byte with OPCODE.
moaxcp · Dec 20, 2023 · 03ebdf5 · 03ebdf5
1 parent 53f900f
commit 03ebdf5
Show file tree

Hide file tree

Showing 18 changed files with 76 additions and 63 deletions.
diff --git a/.idea/compiler.xml b/.idea/compiler.xml
diff --git a/.idea/misc.xml b/.idea/misc.xml
diff --git a/.idea/modules.xml b/.idea/modules.xml
diff --git a/.idea/modules/x11-client.test.iml b/.idea/modules/x11-client.test.iml
diff --git a/README.md b/README.md
@@ -22,15 +22,15 @@ This library can be added to your project using maven or gradle.
 <dependency>
  <groupId>com.github.moaxcp.x11</groupId>
  <artifactId>x11-client</artifactId>
- <version>0.12.0</version>
+ <version>0.14.0</version>
  <type>module</type>
 </dependency>
 ```
 
 ## Gradle
 
 ```
-implementation 'com.github.moaxcp.x11:x11-client:0.12.0'
+implementation 'com.github.moaxcp.x11:x11-client:0.14.0'
 ```
 
 The library has one dependency for using unix sockets.
@@ -452,6 +452,11 @@ https://jamey.thesharps.us/2021/03/25/xcb-protocol-specifications-data/
 
 # versions
 
+## 0.14.0
+
+* Fixed issue where WILD XAuthority entries do not have an address
+* Fixed major bug when writing requests. For xproto the header should start with the OPCODE then 1 pad. For extensions the header should start with the majorOpcode then OPCODE. BigRequest and all QueryVersion requests worked because their OPCODE is 0 but everything else in extensions failed. This should be fixed for all extensions and has been tested with the RECORD extension.
+
 ## 0.13.0
 
 ProtocolPluginService now sets majorOpcode on XProtocolPlugins and uses it instead of majorVersion as the base opcode for requests. This fixes a bug in loading plugins and sending requests.

diff --git a/build.gradle b/build.gradle
@@ -10,7 +10,7 @@ plugins {
     id 'jacoco'
 }
 
-version = '0.13.0'
+version = '0.14.0'
 group = 'com.github.moaxcp.x11'
 description = 'An x11 client implemented in java'
 

diff --git a/buildSrc/src/main/groovy/com/github/moaxcp/x11protocol/xcbparser/JavaRequest.groovy b/buildSrc/src/main/groovy/com/github/moaxcp/x11protocol/xcbparser/JavaRequest.groovy
@@ -58,7 +58,7 @@ class JavaRequest extends JavaClass {
         javaRequest.protocol = request.toJavaProtocol(javaRequest)
         JavaProperty c = javaRequest.getJavaProperty('OPCODE')
         c.constantField = true
-        c.writeValueExpression = CodeBlock.of('(byte)($1T.toUnsignedInt(OPCODE) + $1T.toUnsignedInt(offset))', ClassName.get('java.lang', 'Byte'))
+        c.writeValueExpression = CodeBlock.of('(byte)($1T.toUnsignedInt(OPCODE))', ClassName.get('java.lang', 'Byte'))
         JavaProperty l = javaRequest.getJavaProperty('length')
         l.writeValueExpression = CodeBlock.of('(short) getLength()')
         return javaRequest
@@ -112,7 +112,7 @@ class JavaRequest extends JavaClass {
 
     @Override
     void addWriteParameters(MethodSpec.Builder methodBuilder) {
-        methodBuilder.addParameter(byte.class, 'offset')
+        methodBuilder.addParameter(byte.class, 'majorOpcode')
     }
 
     @Override

diff --git a/buildSrc/src/main/groovy/com/github/moaxcp/x11protocol/xcbparser/XTypeRequest.groovy b/buildSrc/src/main/groovy/com/github/moaxcp/x11protocol/xcbparser/XTypeRequest.groovy
@@ -15,20 +15,26 @@ class XTypeRequest extends XTypeObject {
     }
 
     static XTypeRequest xTypeRequest(XResult result, Node node) {
-        XTypeRequest request = new XTypeRequest(result: result, name: node.attributes().get('name'), opCode: Integer.valueOf((String) node.attributes().get('opcode')), basePackage: result.basePackage, javaPackage: result.javaPackage)
+        int opCode = Integer.valueOf((String) node.attributes().get('opcode')) ?: 0
+        XTypeRequest request = new XTypeRequest(result: result, name: node.attributes().get('name'), opCode: opCode, basePackage: result.basePackage, javaPackage: result.javaPackage)
         request.addUnits(result, node)
-        request.protocol.add(0, new XUnitField(result: result, name: 'OPCODE', type:'CARD8', constantValue: request.opCode))
-        if(request.protocol.size() == 1) {
-            request.protocol.add(new XUnitPad(bytes: 1))
-            request.protocol.add(new XUnitField(result: result, name: 'length', type: 'CARD16', localOnly: true))
-        } else {
-            XUnit firstField = request.protocol[1]
-            if(firstField instanceof XUnitPad && firstField.bytes != 1) {
-                request.protocol.add(1, new XUnitPad(bytes: 1))
-            } else if(firstField instanceof XUnitField && !['BOOL', 'BYTE', 'INT8', 'CARD8', 'char', 'void'].contains(firstField.resolvedType.name)) {
-                request.protocol.add(1, new XUnitPad(bytes: 1))
+        if (result.header == 'xproto') {
+            request.protocol.add(0, new XUnitField(result: result, name: 'OPCODE', type:'CARD8', constantValue: request.opCode))
+            if(request.protocol.size() == 1) {
+                request.protocol.add(new XUnitPad(bytes: 1))
+            } else {
+                XUnit firstField = request.protocol[1]
+                if(firstField instanceof XUnitPad && firstField.bytes != 1) {
+                    request.protocol.add(1, new XUnitPad(bytes: 1))
+                } else if(firstField instanceof XUnitField && !['BOOL', 'BYTE', 'INT8', 'CARD8', 'char', 'void'].contains(firstField.resolvedType.name)) {
+                    request.protocol.add(1, new XUnitPad(bytes: 1))
+                }
             }
             request.protocol.add(2, new XUnitField(result: result, name: 'length', type: 'CARD16', localOnly: true))
+        } else {
+            request.protocol.add(0, new XUnitField(result: result, name: 'majorOpcode', type:'CARD8', localOnly: true))
+            request.protocol.add(1, new XUnitField(result: result, name: 'OPCODE', type:'CARD8', constantValue: request.opCode))
+            request.protocol.add(2, new XUnitField(result: result, name: 'length', type: 'CARD16', localOnly: true))
         }
 
         Node replyNode = node.childNodes().find { it.name() == 'reply' }

diff --git a/buildSrc/src/test/groovy/com/github/moaxcp/x11protocol/XmlSpec.groovy b/buildSrc/src/test/groovy/com/github/moaxcp/x11protocol/XmlSpec.groovy
@@ -21,6 +21,12 @@ abstract class XmlSpec extends Specification {
         parser.parseXml()
     }
 
+    void parseHeaderAndXml() {
+        XParser parser = new XParser(xml: getGPathResult(), result: result)
+        parser.parseHeader()
+        parser.parseXml()
+    }
+
     Node getFirstChild() {
         (Node) getGPathResult().childNodes().next()
     }

diff --git a/buildSrc/src/test/groovy/com/github/moaxcp/x11protocol/xcbparser/JavaReplySpec.groovy b/buildSrc/src/test/groovy/com/github/moaxcp/x11protocol/xcbparser/JavaReplySpec.groovy
@@ -5,7 +5,7 @@ import com.github.moaxcp.x11protocol.XmlSpec
 class JavaReplySpec extends XmlSpec {
     def queryTree() {
         given:
-        xmlBuilder.xcb() {
+        xmlBuilder.xcb(header: 'xproto') {
             xidtype(name:'WINDOW')
             request(name:'QueryTree', opcode:'15') {
                 pad(bytes:'1')
@@ -58,9 +58,9 @@ class JavaReplySpec extends XmlSpec {
               }
             
               @java.lang.Override
-              public void write(byte offset, com.github.moaxcp.x11client.protocol.X11Output out) throws
+              public void write(byte majorOpcode, com.github.moaxcp.x11client.protocol.X11Output out) throws
                   java.io.IOException {
-                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE) + java.lang.Byte.toUnsignedInt(offset)));
+                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE)));
                 out.writePad(1);
                 out.writeCard16((short) getLength());
                 out.writeCard32(window);

diff --git a/buildSrc/src/test/groovy/com/github/moaxcp/x11protocol/xcbparser/JavaRequestSpec.groovy b/buildSrc/src/test/groovy/com/github/moaxcp/x11protocol/xcbparser/JavaRequestSpec.groovy
@@ -42,9 +42,9 @@ class JavaRequestSpec extends XmlSpec {
               }
             
               @java.lang.Override
-              public void write(byte offset, com.github.moaxcp.x11client.protocol.X11Output out) throws
+              public void write(byte majorOpcode, com.github.moaxcp.x11client.protocol.X11Output out) throws
                   java.io.IOException {
-                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE) + java.lang.Byte.toUnsignedInt(offset)));
+                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE)));
                 out.writePad(1);
                 out.writeCard16((short) getLength());
                 out.writeCard32(window);
@@ -141,9 +141,9 @@ class JavaRequestSpec extends XmlSpec {
               }
             
               @java.lang.Override
-              public void write(byte offset, com.github.moaxcp.x11client.protocol.X11Output out) throws
+              public void write(byte majorOpcode, com.github.moaxcp.x11client.protocol.X11Output out) throws
                   java.io.IOException {
-                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE) + java.lang.Byte.toUnsignedInt(offset)));
+                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE)));
                 out.writeByte(coordinateMode);
                 out.writeCard16((short) getLength());
                 out.writeCard32(drawable);
@@ -249,9 +249,9 @@ class JavaRequestSpec extends XmlSpec {
               }
             
               @java.lang.Override
-              public void write(byte offset, com.github.moaxcp.x11client.protocol.X11Output out) throws
+              public void write(byte majorOpcode, com.github.moaxcp.x11client.protocol.X11Output out) throws
                   java.io.IOException {
-                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE) + java.lang.Byte.toUnsignedInt(offset)));
+                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE)));
                 out.writeBool(getOddLength());
                 out.writeCard16((short) getLength());
                 out.writeCard32(font);
@@ -291,39 +291,39 @@ class JavaRequestSpec extends XmlSpec {
         }
 
         when:
-        addChildNodes()
+        parseHeaderAndXml()
         XTypeRequest request = result.resolveXType('Enable')
         JavaRequest javaRequest = request.javaType
 
         then:
         javaRequest.typeSpec.toString() == '''\
             @lombok.Value
             @lombok.Builder
-            public class Enable implements com.github.moaxcp.x11client.protocol.TwoWayRequest<com.github.moaxcp.x11client.protocol.xproto.EnableReply>, com.github.moaxcp.x11client.protocol.xproto.XprotoObject {
+            public class Enable implements com.github.moaxcp.x11client.protocol.TwoWayRequest<com.github.moaxcp.x11client.protocol.bigreq.EnableReply>, com.github.moaxcp.x11client.protocol.bigreq.BigreqObject {
               public static final byte OPCODE = 0;
             
-              public com.github.moaxcp.x11client.protocol.XReplyFunction<com.github.moaxcp.x11client.protocol.xproto.EnableReply> getReplyFunction(
+              public com.github.moaxcp.x11client.protocol.XReplyFunction<com.github.moaxcp.x11client.protocol.bigreq.EnableReply> getReplyFunction(
                   ) {
-                return (field, sequenceNumber, in) -> com.github.moaxcp.x11client.protocol.xproto.EnableReply.readEnableReply(field, sequenceNumber, in);
+                return (field, sequenceNumber, in) -> com.github.moaxcp.x11client.protocol.bigreq.EnableReply.readEnableReply(field, sequenceNumber, in);
               }
             
               public byte getOpCode() {
                 return OPCODE;
               }
             
-              public static com.github.moaxcp.x11client.protocol.xproto.Enable readEnable(
+              public static com.github.moaxcp.x11client.protocol.bigreq.Enable readEnable(
                   com.github.moaxcp.x11client.protocol.X11Input in) throws java.io.IOException {
-                com.github.moaxcp.x11client.protocol.xproto.Enable.EnableBuilder javaBuilder = com.github.moaxcp.x11client.protocol.xproto.Enable.builder();
-                byte[] pad1 = in.readPad(1);
+                com.github.moaxcp.x11client.protocol.bigreq.Enable.EnableBuilder javaBuilder = com.github.moaxcp.x11client.protocol.bigreq.Enable.builder();
+                byte majorOpcode = in.readCard8();
                 short length = in.readCard16();
                 return javaBuilder.build();
               }
             
               @java.lang.Override
-              public void write(byte offset, com.github.moaxcp.x11client.protocol.X11Output out) throws
+              public void write(byte majorOpcode, com.github.moaxcp.x11client.protocol.X11Output out) throws
                   java.io.IOException {
-                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE) + java.lang.Byte.toUnsignedInt(offset)));
-                out.writePad(1);
+                out.writeCard8(majorOpcode);
+                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE)));
                 out.writeCard16((short) getLength());
               }
             
@@ -675,9 +675,9 @@ class JavaRequestSpec extends XmlSpec {
               }
             
               @java.lang.Override
-              public void write(byte offset, com.github.moaxcp.x11client.protocol.X11Output out) throws
+              public void write(byte majorOpcode, com.github.moaxcp.x11client.protocol.X11Output out) throws
                   java.io.IOException {
-                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE) + java.lang.Byte.toUnsignedInt(offset)));
+                out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE)));
                 out.writeCard8(depth);
                 out.writeCard16((short) getLength());
                 out.writeCard32(wid);

diff --git a/buildSrc/src/test/groovy/com/github/moaxcp/x11protocol/xkb/SetCompatMapSpec.groovy b/buildSrc/src/test/groovy/com/github/moaxcp/x11protocol/xkb/SetCompatMapSpec.groovy
@@ -101,9 +101,9 @@ class SetCompatMapSpec extends XmlSpec {
           }
         
           @java.lang.Override
-          public void write(byte offset, com.github.moaxcp.x11client.protocol.X11Output out) throws
+          public void write(byte majorOpcode, com.github.moaxcp.x11client.protocol.X11Output out) throws
               java.io.IOException {
-            out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE) + java.lang.Byte.toUnsignedInt(offset)));
+            out.writeCard8((byte)(java.lang.Byte.toUnsignedInt(OPCODE)));
             out.writePad(1);
             out.writeCard16((short) getLength());
             out.writeCard16(deviceSpec);

diff --git a/src/integrationTest/java/com/github/moaxcp/x11client/RecordIT.java b/src/integrationTest/java/com/github/moaxcp/x11client/RecordIT.java
@@ -1,6 +1,8 @@
 package com.github.moaxcp.x11client;
 
+import com.github.moaxcp.x11client.protocol.X11OutputStream;
 import com.github.moaxcp.x11client.protocol.record.*;
+import java.io.ByteArrayOutputStream;
 import lombok.extern.java.Log;
 import org.junit.jupiter.api.Test;
 
@@ -37,6 +39,11 @@ void test() {
                     .ranges(Collections.singletonList(range))
                     .build();
 
+            try (ByteArrayOutputStream baos = new ByteArrayOutputStream(); X11OutputStream out = new X11OutputStream(baos)) {
+                createContext.write((byte) -110, out);
+                log.info(String.format("createContext bytes size = %d", baos.toByteArray().length));
+            }
+
             log.info(String.format("createContext: %s, size: %d, length: %d", createContext, createContext.getSize(), createContext.getLength()));
 
             client.send(createContext);

diff --git a/src/main/java/com/github/moaxcp/x11client/protocol/X11InputStream.java b/src/main/java/com/github/moaxcp/x11client/protocol/X11InputStream.java
@@ -7,7 +7,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
-public class X11InputStream implements X11Input {
+public class X11InputStream implements X11Input, AutoCloseable {
   private final DataInputStream in;
 
   public X11InputStream(InputStream inputStream) {

diff --git a/src/main/java/com/github/moaxcp/x11client/protocol/X11OutputStream.java b/src/main/java/com/github/moaxcp/x11client/protocol/X11OutputStream.java
@@ -5,7 +5,7 @@
 import java.io.OutputStream;
 import java.util.List;
 
-public class X11OutputStream implements X11Output {
+public class X11OutputStream implements X11Output, AutoCloseable {
   private final DataOutputStream out;
 
   public X11OutputStream(OutputStream outputStream) {

diff --git a/src/main/java/com/github/moaxcp/x11client/protocol/XAuthority.java b/src/main/java/com/github/moaxcp/x11client/protocol/XAuthority.java
@@ -32,12 +32,17 @@ public class XAuthority {
   @ToString
   public enum Family {
     INTERNET(0),
+    DEC_NET(1),
+    CHAOS(2),
+    SERVER_INTERPRETED(5),
+    INTERNET6(6),
     LOCAL(256),
     WILD(65535),
-    KRB5PRINCIPAL(254),
+    NETNAME(254),
+    KRB5PRINCIPAL(253),
     LOCALHOST(252);
 
-    private int code;
+    private final int code;
 
     Family(int code) {
       this.code = code;
@@ -79,7 +84,7 @@ public static Family getByCode(int code) {
    */
   public XAuthority(@NonNull Family family, @NonNull List<Byte> address, int displayNumber, @NonNull List<Byte> protocolName, @NonNull List<Byte> protocolData) {
     this.family = family;
-    this.address = requireNonEmpty("address", address);
+    this.address = address;
     if(displayNumber < 0) {
       throw new IllegalArgumentException("displayNumber was \"" + displayNumber + "\" expected >= 0.");
     }

diff --git a/src/test/java/com/github/moaxcp/x11client/protocol/XAuthorityFamilyTest.java b/src/test/java/com/github/moaxcp/x11client/protocol/XAuthorityFamilyTest.java
@@ -22,7 +22,7 @@ void wildCode() {
   }
   @Test
   void krb5principalCode() {
-    AssertionsForClassTypes.assertThat(Family.KRB5PRINCIPAL.getCode()).isEqualTo(254);
+    AssertionsForClassTypes.assertThat(Family.KRB5PRINCIPAL.getCode()).isEqualTo(253);
   }
   @Test
   void localhostCode() {
@@ -46,7 +46,7 @@ void wild_getByCode() {
 
   @Test
   void krb_getByCode() {
-    assertThat(Family.getByCode(254)).isEqualTo(Family.KRB5PRINCIPAL);
+    assertThat(Family.getByCode(253)).isEqualTo(Family.KRB5PRINCIPAL);
   }
 
   @Test

diff --git a/src/test/java/com/github/moaxcp/x11client/protocol/XAuthorityTest.java b/src/test/java/com/github/moaxcp/x11client/protocol/XAuthorityTest.java
@@ -26,12 +26,6 @@ void constructor_fails_on_null_address() {
     assertThat(exception).hasMessage("address is marked non-null but is null");
   }
 
-  @Test
-  void constructor_fails_on_empty_address() {
-    IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> new XAuthority(LOCAL, toList(new byte[]{}), 0, toList("magic".getBytes()), toList(new byte[] {1})));
-    assertThat(exception).hasMessage("address must not be empty");
-  }
-
   @Test
   void constructor_fails_on_negative_displayNumber() {
     IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> new XAuthority(LOCAL, toList("host".getBytes()), -1, toList("magic".getBytes()), toList(new byte[] {1})));