Fix console log character encoding #4044

ketan · 2017-11-30T15:20:51Z

jyotisingh · 2017-12-07T05:48:40Z

agent/src/com/thoughtworks/go/agent/AgentWebSocketClientController.java

@@ -166,7 +166,8 @@ private void runBuild(BuildSettings buildSettings) {
            buildConsole = new ConsoleOutputTransmitter(
                new RemoteConsoleAppender(
                    urlService.prefixPartialUrl(buildSettings.getConsoleUrl()),
-                    httpService)
+                    httpService,
+                    getSystemEnvironment().consoleLogCharsetAsCharset())


This would be set at the server side not agent. The charset that was passed along from the server to the agent needs to be passed along to the Console appender

This needs to be handled for the BuildCompose workflow. We should be able to pass along the encoding from server as a part of BuildSettings instead of using getSystemEnvironment().consoleLogCharsetAsCharset().

jyotisingh

Did a quick round of testing for this since I wasn't sure if console logs over websocket will work out of the box as far as encoding is concerned. It doesn't. Haven't debugged to see what exactly is the issue with that though.
When I say it doesn't work, I meant the encoding gets screwed up during the transmission. There might be some other issues with transmission of logs over websockets, I wasn't talking about them.

jyotisingh · 2017-12-08T07:34:06Z

common/src/com/thoughtworks/go/publishers/GoArtifactsManipulator.java

@@ -212,6 +212,6 @@ public void setProperty(JobIdentifier jobIdentifier, Property property) {
    public ConsoleOutputTransmitter createConsoleOutputTransmitter(JobIdentifier jobIdentifier,
                                                                   AgentIdentifier agentIdentifier) {
        String consoleUrl = urlService.getUploadUrlOfAgent(jobIdentifier, getConsoleOutputFolderAndFileNameUrl());
-        return new ConsoleOutputTransmitter(new RemoteConsoleAppender(consoleUrl, httpService));
+        return new ConsoleOutputTransmitter(new RemoteConsoleAppender(consoleUrl, httpService, new SystemEnvironment().consoleLogCharsetAsCharset()));


Since BuildWork already contains the charset from server, we should just be able to pass it here.

jyotisingh · 2017-12-08T07:46:09Z

agent/src/com/thoughtworks/go/agent/AgentWebSocketClientController.java

@@ -166,7 +166,8 @@ private void runBuild(BuildSettings buildSettings) {
            buildConsole = new ConsoleOutputTransmitter(
                new RemoteConsoleAppender(
                    urlService.prefixPartialUrl(buildSettings.getConsoleUrl()),
-                    httpService)
+                    httpService,
+                    getSystemEnvironment().consoleLogCharsetAsCharset())


This needs to be handled for the BuildCompose workflow. We should be able to pass along the encoding from server as a part of BuildSettings instead of using getSystemEnvironment().consoleLogCharsetAsCharset().

ketan · 2018-02-06T06:04:27Z

common/src/main/java/com/thoughtworks/go/buildsession/ExecCommandExecutor.java

@@ -83,6 +84,7 @@ private CommandLine createCommandLine(String cmd) {
        } else {
            commandLine = CommandLine.createCommandLine(cmd);
        }
+        commandLine.withEncoding(new SystemEnvironment().consoleLogCharset());


@jyotisingh — we'd likely need to inject the charset to all BuildCommandExecutor impls, or add it to BuildSession to make it available to all executors.

Thoughts?

I prefer adding it to the BuildSession. One way would be to pass along the charset with BuildSettings and then use it to construct the BuildSession. That way in future too if any other CommandExecutor needs access to commandline, it could just get hold of the charset from BuildSession

As part of commit 7fc200b, the `Content-Type` header was set to `text/plain; charset=utf-8`. However the `HttpServletResponse`'s `getWriter()` was returning a writer bound to the system default character encoding. We now force the encoding of the writer to `utf-8`. Fixes gocd#3926, gocd#3442

Charset is determined on the server using the system property `go.console.log.charset`.

ketan · 2018-02-07T11:16:13Z

For testing this PR, create a file (/tmp/unicode.txt) with some unicode characters, emojis. I used this file, make sure to click "raw" and download using wget, copy/pasting is not recommended.

Use the following config.xml snippet:

    <pipeline name="up42">
      <materials>
        <git url="test-repo" />
      </materials>
      <stage name="up42_stage">
        <jobs>
          <job name="up42_job">
            <tasks>
              <exec command="bash" args="-c 'while true; do cat /tmp/unicode.txt; sleep 1; done'" />
            </tasks>
          </job>
        </jobs>
      </stage>
    </pipeline>

On 18.1 of GoCD server and agent:

set the file.encoding system property to cp1252 (to simulate GoCD running on windows). This needs to be done on both the server and agent.
Run the build
When viewing the console logs, the non-ascii part the output will appear garbled

On latest (experimental) GoCD

set the file.encoding system property to cp1252 (to simulate GoCD running on windows). This needs to be done on both the server and agent.
Run the build
the console output will not be garbled

On latest (experimental) GoCD

set the go.console.log.charset system property to cp1252 (to force console log encoding to be that used by windows). This needs to be done on the server, doing it on the agent has no effect
Run the build
the console output will be garbled — because the unicode text is being rendered using cp1252 encoding. This is expected behaviour.

adityasood · 2018-02-07T13:48:12Z

Tested the above steps

On 18.1 of GoCD server and agent:
The non-ascii part the output appeared garbled after file.encoding is set to cp1252 on both GoCD server and agent
On latest (experimental) GoCD git sha (6029...)
The console output is not garbled after file.encoding is set to cp1252 on both GoCD server and agent
On latest (experimental) GoCD git sha (6029...)
The console output is garbled after go.console.log.charset is set to cp1252 on GoCD server

lobster2012-user · 2020-01-06T11:04:45Z

It works for me.

wrapper.java.additional.100=-Dgo.console.log.charset=CP866

ketan force-pushed the fix/console-log-char-encoding branch 18 times, most recently from 2ee601d to 79b9e5d Compare December 4, 2017 10:21

ketan mentioned this pull request Dec 5, 2017

Bug: Console logs stop updating when non-ascii characters are encountered #3926

Closed

ketan force-pushed the fix/console-log-char-encoding branch from 79b9e5d to a99d30d Compare December 7, 2017 05:45

jyotisingh reviewed Dec 7, 2017

View reviewed changes

ketan force-pushed the fix/console-log-char-encoding branch 2 times, most recently from f9ea049 to 8ae3f7f Compare December 11, 2017 03:58

jyotisingh suggested changes Dec 11, 2017

View reviewed changes

ketan force-pushed the fix/console-log-char-encoding branch 4 times, most recently from 4b62934 to 2a43292 Compare December 21, 2017 08:59

varshavaradarajan mentioned this pull request Jan 12, 2018

Missing Emoji support between 17.4 and 17.12 #4178

Closed

ketan force-pushed the fix/console-log-char-encoding branch from 2a43292 to eb3087d Compare February 6, 2018 05:00

ketan force-pushed the fix/console-log-char-encoding branch 5 times, most recently from 71cfe71 to 5971481 Compare February 6, 2018 05:34

ketan commented Feb 6, 2018

View reviewed changes

ketan force-pushed the fix/console-log-char-encoding branch 5 times, most recently from 7ad694a to 6c8a9c8 Compare February 6, 2018 14:37

ketan added this to the Release 18.2 milestone Feb 6, 2018

ketan force-pushed the fix/console-log-char-encoding branch from 6c8a9c8 to dab2c7c Compare February 7, 2018 11:00

ketan added 5 commits February 7, 2018 16:30

Use utf-8 for all scm commands

6abaa15

Allow console log charset to be configurable via system property

cf68dc3

Blow up StreamPumper if encoding is not specified

7744b2e

Charset is determined on the server using the system property `go.console.log.charset`.

Force charset on buildsession

dab2c7c

ketan merged commit dab2c7c into gocd:master Feb 7, 2018

ketan deleted the fix/console-log-char-encoding branch February 7, 2018 11:00

ketan mentioned this pull request May 30, 2018

Force file.encoding to utf-8 #3068

Closed

arvindsv mentioned this pull request Jan 6, 2020

It is not possible to specify the encoding in the console. #7537

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix console log character encoding #4044

Fix console log character encoding #4044

ketan commented Nov 30, 2017

jyotisingh Dec 7, 2017

jyotisingh Dec 8, 2017

jyotisingh left a comment •

edited

Loading

jyotisingh Dec 8, 2017

jyotisingh Dec 8, 2017

ketan Feb 6, 2018

jyotisingh Feb 6, 2018

ketan commented Feb 7, 2018

adityasood commented Feb 7, 2018

lobster2012-user commented Jan 6, 2020

Fix console log character encoding #4044

Fix console log character encoding #4044

Conversation

ketan commented Nov 30, 2017

jyotisingh Dec 7, 2017

Choose a reason for hiding this comment

jyotisingh Dec 8, 2017

Choose a reason for hiding this comment

jyotisingh left a comment • edited Loading

Choose a reason for hiding this comment

jyotisingh Dec 8, 2017

Choose a reason for hiding this comment

jyotisingh Dec 8, 2017

Choose a reason for hiding this comment

ketan Feb 6, 2018

Choose a reason for hiding this comment

jyotisingh Feb 6, 2018

Choose a reason for hiding this comment

ketan commented Feb 7, 2018

On 18.1 of GoCD server and agent:

On latest (experimental) GoCD

On latest (experimental) GoCD

adityasood commented Feb 7, 2018

lobster2012-user commented Jan 6, 2020

jyotisingh left a comment •

edited

Loading