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.

Sign up for GitHub

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

Jump to bottom

NamespaceBinding.buildString inproper handling of TopScope #45

Closed
muntis opened this issue Jan 6, 2015 · 2 comments
Closed

NamespaceBinding.buildString inproper handling of TopScope #45

muntis opened this issue Jan 6, 2015 · 2 comments

Comments

@muntis
Copy link

muntis commented Jan 6, 2015

Starting with saca 2.11 NamespaceBinding.buildString uses private metohod doBuildString for namespace part generation. problem is that this method is not overrided in TopScope sub class. This generates unnecessary artifacts in form of empty (xmlns="") when constructing xml:

  def pretty(xml: Node) = new scala.xml.PrettyPrinter(200, 2).format(xml)
  val xx = <soapenv:Envelope xmlns:soapenv="aaa">
    <soapenv:Header xmlns:wsa="ssss">
    </soapenv:Header>
    <soapenv:Body><body xmlns:soapenv="aaa"></body></soapenv:Body>
  </soapenv:Envelope>
  println(pretty(xx))

Results in:

<soapenv:Envelope xmlns:soapenv="aaa" xmlns="">
  <soapenv:Header xmlns:wsa="ssss"> </soapenv:Header>
  <soapenv:Body>
    <body xmlns:soapenv="aaa"></body>
  </soapenv:Body>
</soapenv:Envelope>

In this example problem can be fixed by setting pscope=TopScope for PrettyPrinter, but it does not help if xml is build from multiple parts.

def pretty(xml: Node) = new scala.xml.PrettyPrinter(200, 2).format(xml, TopScope)
val body = <body xmlns:soapenv="aaa"></body>
val xx = <soapenv:Envelope xmlns:soapenv="aaa">
    <soapenv:Header xmlns:wsa="ssss">
    </soapenv:Header>
    <soapenv:Body>{body}</soapenv:Body>
    <soapenv:Body><body xmlns:soapenv="aaa"></body></soapenv:Body>
  </soapenv:Envelope>
println(pretty(xx))

Results in:

<soapenv:Envelope xmlns:soapenv="aaa">
  <soapenv:Header xmlns:wsa="ssss"> </soapenv:Header>
  <soapenv:Body>
    <body xmlns:soapenv="aaa" xmlns=""></body>
  </soapenv:Body>
  <soapenv:Body>
    <body xmlns:soapenv="aaa"></body>
  </soapenv:Body>
</soapenv:Envelope>

Sugested solution:

override def buildString(sb: StringBuilder, stop: NamespaceBinding) {
  NamespaceBinding.doBuildString(shadowRedefined(stop), sb, stop)
}


object NamespaceBinding{
  def doBuildString(b: NamespaceBinding, sb: StringBuilder, stop: NamespaceBinding) {
    if ((this == null) || (this eq stop)) return // contains?

    sb append " xmlns%s=\"%s\"".format(
      (if (b.prefix != null) ":" + b.prefix else ""),
      (if (b.uri != null) b.uri else "")
    )
    b.parent match {
      case binding: NamespaceBinding => doBuildString(binding, sb, stop)
      case TopScope =>
    }
  }
}

Or just change visibility of doBuildString and override it properly in TopScope like it's done with buildString methods
@muntis muntis mentioned this issue Jan 12, 2015
Closed
@ashawley
Copy link
Member

The other option is adding TopScope as one of the sentinel values checked so that the function is prematurely exited and the blank namespace attribute is never built.

This is a strange bug. It is only complicated by the null-value problem. This is similar to issue #28 that I ran across. At some point, null was allowed and supported as a namespace binding throughout scala-xml. Later, null was abandoned -- in favor of the TopScope construct. Evidence the constructor for Elem which alludes to this transition from null to TopScope:

scala> scala.xml.Elem("x", "foo", scala.xml.Null, null, true)
java.lang.IllegalArgumentException: scope is null, use scala.xml.TopScope for empty scope

This bug has nothing to do with the PrettyPrinter. Even the standard XML toString method is broken. I'll try to add a slightly simpler test case, and then the fix using TopScope.

ashawley added a commit to ashawley/scala-xml that referenced this issue Jan 28, 2015
@ashawley
Nested XML adds empty xmlns attribute scala#45
596f183 
	* src/test/scala/scala/xml/XMLTest.scala (namespacesWithNestedXmls):
	Modified test case by Muntis Grūbe.

	* src/main/scala/scala/xml/NamespaceBinding.scala (doBuildString):
	Add TopScope as a value to avoid building the namespace attribute
	string.
biswanaths pushed a commit that referenced this issue Apr 14, 2016
@ashawley @biswanaths
Nested XML adds empty xmlns attribute #45
ab07818 
	* src/test/scala/scala/xml/XMLTest.scala (namespacesWithNestedXmls):
	Modified test case by Muntis Grūbe.

	* src/main/scala/scala/xml/NamespaceBinding.scala (doBuildString):
	Add TopScope as a value to avoid building the namespace attribute
	string.
@biswanaths
Copy link
Contributor

Thanks Aaron. Closing this one.

@adriaanm adriaanm mentioned this issue Apr 22, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@biswanaths @ashawley @muntis