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

XML response rendering indentation algorithm problem #1918

Closed
andylowry opened this issue Jan 26, 2016 · 4 comments
Closed

XML response rendering indentation algorithm problem #1918

andylowry opened this issue Jan 26, 2016 · 4 comments
Milestone

Comments

@andylowry
Copy link

In a "Try It Now" scenario, if the response contains XML content, the rendering algorithm may fail to indent the content properly in its rendering, due to greedy regex matching.

Here's an example:

The content supplied by the web service is:

<Person version="1.0">
  <taxpayerID>ssn-xx-xxxx</taxpayerID>
  <lastName>Eliot</lastName>
  <firstName>George</firstName>
  <otherNames>Mary Ann Evans</otherNames>
</Person>

The rendering in swagger-ui is:

<Person version="1.0">
  <taxpayerID>ssn-xx-xxxx</taxpayerID>
    <lastName>Eliot</lastName>
      <firstName>George</firstName>
        <otherNames>Mary Ann Evans</otherNames>
        </Person>

As can be seen, each line of the form <xxx>...</xxx> causes the next line to be indented progressively further than itself. This is due, I believe, to greedy matching in regular expressions in the rendering code.

The code appears to operate something like this:

  1. Preprocess the content to insert newlines so that open, close and self-closing tags all appear on their own lines, so the next phase makes sense.
  2. Go line-by-line through the content, rendering each line with the prevailing indentation
  3. When processing each line, determine its "type" by looking for the first embedded tag, and use transistions between line types to adjust prevailing indentation.

The preprocessing regular expressions are faulty, e.g. they're not splitting the tags out of the above lines because of greedy matches. For example, consider this pattern:

    contexp = /(<.+>)(.+\n)/g;

It is used to insert newlines after tags. However, to this pattern, a line like <xxx>...</xxx> looks like one big tag. If the pattern were non-greedy (/(<.+?>)(.*\n)/g) this would not be the case. (Note that I've also changed + to * near the end of the pattern to fix another minor bug - there's really no requirement for another char before the newline here, is there?)

Later, the line-typing algorithm will again use greedy matches and consider this line to be of type "opening" which causes the prevailing indentation to be increased by 1.

Hopefully this is enough for the right person to fix this code. I apologize that I'm not currently able to spare the time to do it myself.

@fehguy
Copy link
Contributor

fehguy commented Jan 27, 2016

@bodnia if you have time, this may be worth checking out

@andylowry
Copy link
Author

Sorry, what I was seeing is real, but my analysis was wrong.

I'm not entirely sure why the contexp regex is correctly causing something of the form <xxx>foo</xxx> to be split into <xxx>\nfoo</xxx> but it is. So that's something maybe for me to study and understand.

But what was really breaking the formatting in my case was that my XML was being produced by Java on windows, and all the line ends were \r\n not just \n. So the wsexp regexp was not trimming lines with trailing spaces, and the contextp regex was not causing the above pattern to be split. (Another one I don't understand - MDN doc says that '.' matches anything but newline character, but it also apparently doesn't match carriage-return?).

It turns out the line typing was categorizing these lines as "closing" not "opening" and so the indentation was going negative. What I hadn't realized is that the indenting code, through some very convoluted logic, computes an indentation string whose length is the absolute value of the indent value. Hence the results I included above.

So bottom line is I can work around this by removing CR chars from my output before passing it on to swagger-ui. But it would probably be helpful for swagger-ui to do this as well, to prevent others hitting this issue.

@fehguy
Copy link
Contributor

fehguy commented Jan 27, 2016

Hi, thanks for the feedback. We want to be equal-opportunity so we'll make sure this is working with documents from windows and others as well.

@webron
Copy link
Contributor

webron commented Jun 9, 2016

Should be fixed now.

@webron webron closed this as completed Jun 9, 2016
Jonahss pushed a commit to eaze/swagger-ui that referenced this issue Aug 12, 2016
vincent-zurczak pushed a commit to roboconf/swagger-ui that referenced this issue Aug 19, 2016
@fehguy fehguy added this to the v2.2.1 milestone Aug 23, 2016
JuanSW18 pushed a commit to Digital-Paw/digital-paw-swagger-ui that referenced this issue Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants