vert-x3 / vertx-rabbitmq-client

Vert.x RabbitMQ Service
Apache License 2.0
73 stars 64 forks source link

Message encoding #89

Closed barmic closed 3 years ago

barmic commented 5 years ago

Hello,

In io.vertx.rabbitmq.impl.Utils (here), we use platform dependant charset for encode message.

For decoding we enforce usage of UTF8 if no encoding given.

Maybe we can have a symmetric solution for encoding?

Have a nice day

vietj commented 5 years ago

how about having this as an option of the client that would be used for encoding / decoding ?

barmic commented 5 years ago

Why not? I try to implement it to create a PR but I'm not fluent with the code generation of vertx and I don't found how implement it.

vietj commented 5 years ago

you don't need to care about code generation

YaytayAtWork commented 3 years ago

Isn't the issue here just that the default for decode is UTF-8, but the default for encode is platform dependent:

  public static String decode(String encoding, byte[] bytes) throws UnsupportedEncodingException {
    if (encoding == null) {
      return new String(bytes, "UTF-8");
    } else {
      return new String(bytes, encoding);
    }
  }

  public static byte[] encode(String encoding, String string) throws UnsupportedEncodingException {
    if (encoding == null) {
      return string.getBytes();
    } else {
      return string.getBytes(encoding);
    }
  }

The default for encode should be changed to match that for decode: string.getBytes("UTF-8"); Though personally I'd prefer both to use the Charset version:

  public static String decode(String encoding, byte[] bytes) throws UnsupportedEncodingException {
    if (encoding == null) {
      return new String(bytes, StandardCharsets.UTF_8);
    } else {
      return new String(bytes, encoding);
    }
  }

  public static byte[] encode(String encoding, String string) throws UnsupportedEncodingException {
    if (encoding == null) {
      return string.getBytes(StandardCharsets.UTF_8);
    } else {
      return string.getBytes(encoding);
    }
  }

If you are happy with the use of StandardCharsets I'll do a PR.

Yaytay commented 3 years ago

Merged