Fix crash that occurs when attempting to retrieve the name property of an XMLDocument by making it return nil.

[ghost]

Runtime crash occurs when one attempts to retrieve XMLDocument's name property. This was due to the lack of checking if Documents were allowed or not to have its name property retrieved.

This came from SR-10776, and a PR has been written, but no tests were run and the PR has not been attended to: #2316.

Unlike the original PR, this PR does not have any changes on XMLDocument in Swift, as there is no need to check for document there, but rather in the _getQName function.

[ghost]

@swift-ci please test

[YOCKOW]

My opinions about #2316 and this PR are:

• The latest tests of SR-10776: Fix runtime crash of calling XMLDocument's var name: String? #2316 did run, but Jenkins just didn't report the results of them.
• SR-10776: Fix runtime crash of calling XMLDocument's var name: String? #2316 is also the first step to resolve SR-10764 not only SR-10776.
• I thought name's behavior should be defined in SwiftFoudation not in CoreFoundation whose CFXML* are just wrappers of libxml2’s struct.
• This PR has too many commits to resolve the issue.
• Why don’t you review SR-10776: Fix runtime crash of calling XMLDocument's var name: String? #2316 before creating a new PR?

Thank you.

[ghost]

The reason why it should be in corefoundation is because that is where, according to the crash report you submitted and the crash that happened on my machine, was. The crash happened in that function, where document was allowed to run unchecked.

[ghost]

I would like to test these changes on here (they passed locally), before I squash the commits just to be sure these bugs are fixed and no new ones are introduced.

[YOCKOW]

The reason why it should be in corefoundation is because that is where, according to the crash report you submitted and the crash that happened on my machine, was. The crash happened in that function, where document was allowed to run unchecked.

Well, sorry, but I talked about the setter.
Moreover, the setter calls only _CFXMLNodeSetName. I think your PR contains unrelated changes.

I would like to test these changes on here (they passed locally), before I squash the commits just to be sure these bugs are fixed and no new ones are introduced.

You can read @xwu’s comments in your own past PR about that:

• Refactored code to remove unnecessary checks #2425 (comment)
• Made methods in CFSocketStream consistent in style #2443 (comment)

[xwu]

@pi1024e:

Again, as a matter of etiquette, it's not cool to take a bug that someone else is clearly still working on with an active pull request; if you're interested in helping to resolve such an issue or disagree with how they've approached the topic, you can review their work and offer to help that person.

@YOCKOW has also pointed out that this PR also contains unrelated changes.

I'm not sure of your level of experience with these things, but these sort of things apply generally to most if not all projects.

[ghost]

I apologize, as many of these bugs appear to be inactive. I am deeply sorry about this.

[millenomi]

As a separate note, I'm less against mixed-content patches so long that all the changes in a patch are either required for the main part of it to function, or strongly needed to improve its ergonomics.

In general, I do not take cleanup patches to Core Foundation code in the summer because it complicates the process of merging that I do every year in the early fall.

[ghost]

Fixed it so it is not unnecessary changes! @millenomi

[millenomi]

I took YOCKOW's patch, per discussion on the patch I closed — if we're targeting 5.0 still, we should cherry-pick that one.

[ghost]

Got it!