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

Implement XML parsing using stdlib #155

Merged
merged 5 commits into from
Nov 11, 2024
Merged

Implement XML parsing using stdlib #155

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
02d8c6a
feat: remove import from github.com/clbanning/mxj/v2 and use encoding…
HarshNarayanJha Nov 2, 2024
8d67026
fix: also parse urls from the attrs of the opening tags of xml
HarshNarayanJha Nov 8, 2024
0898a6c
fix: check if token decoding is valid or EOF
HarshNarayanJha Nov 8, 2024
bd06065
fix: directly use bytes reading for xmlBody
HarshNarayanJha Nov 8, 2024
f1f9575
chore: remove unused leafnodes array and element tracking
HarshNarayanJha Nov 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 51 additions & 13 deletions internal/pkg/crawl/extractor/xml.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package extractor

import (
"encoding/xml"
"io"
"net/http"
"net/url"
"strings"

"github.com/clbanning/mxj/v2"
)

type LeafNode struct {
Path string `json:"path"`
Value string `json:"value"`
}

CorentinB marked this conversation as resolved.
Show resolved Hide resolved
func XML(resp *http.Response) (URLs []*url.URL, sitemap bool, err error) {
xmlBody, err := io.ReadAll(resp.Body)
if err != nil {
Expand All @@ -19,25 +23,59 @@ func XML(resp *http.Response) (URLs []*url.URL, sitemap bool, err error) {
sitemap = true
}

mv, err := mxj.NewMapXml(xmlBody)
reader := strings.NewReader(string(xmlBody))
CorentinB marked this conversation as resolved.
Show resolved Hide resolved
decoder := xml.NewDecoder(reader)

var (
startElement xml.StartElement
currentNode *LeafNode
leafNodes []LeafNode
)

// try to decode one token to see if stream is open
_, err = decoder.Token()
if err != nil {
return nil, sitemap, err
}

// Try to find if it's a sitemap
for _, node := range mv.LeafNodes() {
if strings.Contains(node.Path, "sitemap") {
sitemap = true
// seek back to 0 if we are still here
reader.Seek(0, 0)
decoder = xml.NewDecoder(reader)

for {
tok, err := decoder.Token()
if err == io.EOF {
break
}
}
if err != nil {
return nil, sitemap, err
}

for _, value := range mv.LeafValues() {
if _, ok := value.(string); ok {
if strings.HasPrefix(value.(string), "http") {
URL, err := url.Parse(value.(string))
switch tok := tok.(type) {
case xml.StartElement:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

		case xml.StartElement:
			startElement = tok
			currentNode = &LeafNode{Path: startElement.Name.Local}
			for _, attr := range tok.Attr {
				if strings.HasPrefix(attr.Value, "http") {
					parsedURL, err := url.Parse(attr.Value)
					if err == nil {
						URLs = append(URLs, parsedURL)
					}
				}
			}

this fixed the Huge sitemap test by extracting XML attributes.
Now, the URLs' size and content match the previous tests. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, opening tags also have urls in some cases.

Now the only left is TestXMLBodyReadError, gotta look at it

Copy link
Contributor

@yzqzss yzqzss Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the TestXMLBodyReadError test is invalid(?) since NopCloser certainly won't return an EOF error on xmlBody, err := io.ReadAll(resp.Body)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible, but they did pass previously. Sure, it does not return an EOF on read. For the test to pass, I had to decode the Token once and catch the error. (and seek back for the loop)

_, err = decoder.Token()
if err != nil {
	return nil, sitemap, err
}

// seek back to 0 if we are still here
reader.Seek(0, 0)
decoder = xml.NewDecoder(reader)

Catching this in the loop won't work cleanly, since I want to know if EOF was somewhere in-between the file (invalid XML), or at the start (this error)

I will push these changes

startElement = tok
currentNode = &LeafNode{Path: startElement.Name.Local}
for _, attr := range tok.Attr {
if strings.HasPrefix(attr.Value, "http") {
parsedURL, err := url.Parse(attr.Value)
if err == nil {
URLs = append(URLs, parsedURL)
}
}
}
case xml.EndElement:
if currentNode != nil {
leafNodes = append(leafNodes, *currentNode)
currentNode = nil
}
case xml.CharData:
if currentNode != nil && len(strings.TrimSpace(string(tok))) > 0 {
currentNode.Value = string(tok)
}
if strings.HasPrefix(string(tok), "http") {
parsedURL, err := url.Parse(string(tok))
if err == nil {
URLs = append(URLs, URL)
URLs = append(URLs, parsedURL)
}
}
}
Expand Down