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.

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

Map in Transfer could break HTML rendering #61 #68

Merged
merged 1 commit into from Aug 28, 2020

Conversation

fredericBregier
Copy link
Collaborator

@fredericBregier fredericBregier commented Aug 11, 2020

Fix Issue #61 by escaping the Map in TransferInfo and unescaping it while using the map

@fredericBregier fredericBregier linked an issue Aug 11, 2020 that may be closed by this pull request
@fredericBregier fredericBregier added this to the 3.4.1 milestone Aug 14, 2020
@fredericBregier fredericBregier added the bug Something isn't working label Aug 14, 2020
@fredericBregier fredericBregier requested a review from bcarlin Aug 17, 2020
Copy link
Member

@bcarlin bcarlin left a comment

These changes indeed fixes the admin transfer listing.

They have, however, an unintended side effect: The json output of the commands have too much excaping:

With the patch applied, I have the following output:

➜ ./bin/waarp-r66client.sh server2 send -file foo.txt -to server1 -rule push
[DirectTransfer] Transfer in status:       SUCCESS 
{"command":"DirectTransfer","args":"etc/conf.d/server2/client.xml -file foo.txt -to server1 -rule push ","status":0,"statusTxt":"Transfer in status:       SUCCESS ","remote":"server1","ruleid":"push","statusCode":"O","specialid":-9223372036854775806,"finalPath":"/out/foo.txt","requested":"server1","requester":"server2","fileInformation":"noinfo {\\\"follow\\\":-7298291395239706531}","originalSize":12,"originalPath":"/out/foo.txt","filefinal":"File: /out/foo.txt Ready true isExternal false 0","delay":10399}

If I want to get the follow id, i will:

  1. parse the main json,
  2. get the value of fileInformation
  3. get rid of "noinfo "
  4. parse the remaining json.

This remaining json is {\"follow\":-7298291395239706531}, taking into account the un-escaping done by the parser of the main document.
This is not valid json, and parsing that fails (ex: https://play.golang.org/p/oeW_sZPI3US)

@fredericBregier
Copy link
Collaborator Author

fredericBregier commented Aug 26, 2020

FileTransfer is not a JSON but a String, containing a JSON, therefore escaped (since " is not valid in a String).
The procedure is that the JSON is the last part (fusion of possible multiple Json) of the final String, and is escaped.
Then it is parsed internally using regex `{[^\}]*} to match.

So this is a correct Json in escaped mode.
WDYT?

@bcarlin
Copy link
Member

bcarlin commented Aug 27, 2020

Wether I extract the Json part of the string with string manipulation or a regex, this has the same result.

The thing is that what is printed on stdout, {\\\"follow\\\":-7298291395239706531} corresponds to the raw string (the underlying value of the String object) {\"follow\":-7298291395239706531}, which cannot be parsed.

The valid JSON would be {"follow":-7298291395239706531}, so its String representation is just {\"follow\":-7298291395239706531}, which explains my "too much escaping comment".

@fredericBregier
Copy link
Collaborator Author

fredericBregier commented Aug 28, 2020

I update this MR. I obtain something like:
{\"follow\":-7298291395239706531} (both on HTML and output)
So single \.
For the correction of FileInfo andTransferInfo, it will be in the last MR since it needs some deep correction (hopefully not too much).
Are you OK with this ?

@fredericBregier fredericBregier requested a review from bcarlin Aug 28, 2020
Fix Issue waarp#61 by escaping the Map in TransferInfo and unescaping it while using the map
The escaping is minimal (single `\`).
@fredericBregier fredericBregier merged commit 40787df into waarp:v3.4 Aug 28, 2020
@fredericBregier fredericBregier deleted the fixHtmlRendering branch Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[R66] Listing pages are broken in the admin interface
2 participants