Skip to content

Commit

Permalink
Fix failing test with invalid port by adding popper handling of the e…
Browse files Browse the repository at this point in the history 
…rror.

Will also handle other reference errors more gracefully.

Closes #127.
  • Loading branch information
@tomtom5152
tomtom5152 committed Dec 2, 2019
1 parent d9b1c7e commit 6d236af
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 36 deletions.
9 changes: 5 additions & 4 deletions htmldoc/reference.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package htmldoc

import (
"github.com/wjdp/htmltest/output"
"golang.org/x/net/html"
"net/url"
"path"
Expand All @@ -19,7 +18,7 @@ type Reference struct {

// NewReference : Create a new reference given a document, node and path.
// Generates the URL object.
func NewReference(document *Document, node *html.Node, path string) *Reference {
func NewReference(document *Document, node *html.Node, path string) (*Reference, error) {
// Clean path
path = strings.TrimLeftFunc(path, invalidPrePostRune)
path = strings.TrimRightFunc(path, invalidPrePostRune)
Expand All @@ -31,9 +30,11 @@ func NewReference(document *Document, node *html.Node, path string) *Reference {
}
// Parse and store parsed URL
u, err := url.Parse(path)
output.CheckErrorPanic(err)
if err != nil {
return nil, err
}
ref.URL = u
return &ref
return &ref, nil
}

// Scheme : Returns the scheme of the reference. Uses URL.Scheme and adds
Expand Down
51 changes: 27 additions & 24 deletions htmldoc/reference_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,32 @@ func TestReferenceScheme(t *testing.T) {

var ref *Reference

ref = NewReference(&doc, nodeElem, "http://test.com")
ref, _ = NewReference(&doc, nodeElem, "http://test.com")
assert.Equals(t, "http reference", ref.Scheme(), "http")
ref = NewReference(&doc, nodeElem, "https://test.com")
ref, _ = NewReference(&doc, nodeElem, "https://test.com")
assert.Equals(t, "https reference", ref.Scheme(), "https")
ref = NewReference(&doc, nodeElem, "//test.com")
ref, _ = NewReference(&doc, nodeElem, "//test.com")
assert.Equals(t, "https reference", ref.Scheme(), "https")
ref = NewReference(&doc, nodeElem,
ref, _ = NewReference(&doc, nodeElem,
"https://photos.smugmug.com/photos/i-CNHsHLM/0/440x622/i-CNHsHLM-440x622.jpg")
assert.Equals(t, "http reference", ref.Scheme(), "https")
ref = NewReference(&doc, nodeElem, "x?a=1#3")
ref, _ = NewReference(&doc, nodeElem, "x?a=1#3")
assert.Equals(t, "file reference", ref.Scheme(), "file")
ref = NewReference(&doc, nodeElem, "#123")
ref, _ = NewReference(&doc, nodeElem, "#123")
assert.Equals(t, "self reference", ref.Scheme(), "self")
ref = NewReference(&doc, nodeElem, "mailto:x@y.com")
ref, _ = NewReference(&doc, nodeElem, "mailto:x@y.com")
assert.Equals(t, "mailto reference", ref.Scheme(), "mailto")
ref = NewReference(&doc, nodeElem, "tel:123")
ref, _ = NewReference(&doc, nodeElem, "tel:123")
assert.Equals(t, "tel reference", ref.Scheme(), "tel")
ref = NewReference(&doc, nodeElem, "abc:123")
ref, _ = NewReference(&doc, nodeElem, "abc:123")
assert.Equals(t, "unknown reference", ref.Scheme(), "")

// Grubby url
ref = NewReference(&doc, nodeElem, "\n http://foo")
var err error
ref, _ = NewReference(&doc, nodeElem, "\n http://foo")
assert.Equals(t, "unknown reference", ref.Scheme(), "http")
_, err = NewReference(&doc, nodeElem, "http://foo:____")
assert.IsTrue(t, "invalid url port", err != nil && strings.Contains(err.Error(), "invalid port \":____\" after host"))
}

func TestReferenceURLString(t *testing.T) {
Expand All @@ -60,13 +63,13 @@ func TestReferenceURLString(t *testing.T) {

var ref *Reference

ref = NewReference(&doc, nodeElem, "http://example.com")
ref, _ = NewReference(&doc, nodeElem, "http://example.com")
assert.Equals(t, "URLString", ref.URLString(), "http://example.com")
ref = NewReference(&doc, nodeElem, "http://example.com/")
ref, _ = NewReference(&doc, nodeElem, "http://example.com/")
assert.Equals(t, "URLString", ref.URLString(), "http://example.com/")
ref = NewReference(&doc, nodeElem, "https://example.com")
ref, _ = NewReference(&doc, nodeElem, "https://example.com")
assert.Equals(t, "URLString", ref.URLString(), "https://example.com")
ref = NewReference(&doc, nodeElem, "//example.com")
ref, _ = NewReference(&doc, nodeElem, "//example.com")
assert.Equals(t, "URLString", ref.URLString(), "https://example.com")

}
Expand All @@ -82,15 +85,15 @@ func TestReferenceIsInternalAbsolute(t *testing.T) {

var ref *Reference

ref = NewReference(&doc, nodeElem, "/abc/page.html")
ref, _ = NewReference(&doc, nodeElem, "/abc/page.html")
assert.IsTrue(t, "internal absolute reference", ref.IsInternalAbsolute())
ref = NewReference(&doc, nodeElem, "/yyz")
ref, _ = NewReference(&doc, nodeElem, "/yyz")
assert.IsTrue(t, "internal absolute reference", ref.IsInternalAbsolute())
ref = NewReference(&doc, nodeElem, "zzy")
ref, _ = NewReference(&doc, nodeElem, "zzy")
assert.IsFalse(t, "internal relative reference", ref.IsInternalAbsolute())
ref = NewReference(&doc, nodeElem, "zzy/uup.jjr")
ref, _ = NewReference(&doc, nodeElem, "zzy/uup.jjr")
assert.IsFalse(t, "internal relative reference", ref.IsInternalAbsolute())
ref = NewReference(&doc, nodeElem, "./zzy/uup.jjr")
ref, _ = NewReference(&doc, nodeElem, "./zzy/uup.jjr")
assert.IsFalse(t, "internal relative reference", ref.IsInternalAbsolute())
}

Expand All @@ -106,15 +109,15 @@ func TestReferenceAbsolutePath(t *testing.T) {

var ref *Reference

ref = NewReference(&doc, nodeElem, "/abc/page.html")
ref, _ = NewReference(&doc, nodeElem, "/abc/page.html")
assert.Equals(t, "internal absolute reference", ref.RefSitePath(), "/abc/page.html")
ref = NewReference(&doc, nodeElem, "/yyz")
ref, _ = NewReference(&doc, nodeElem, "/yyz")
assert.Equals(t, "internal absolute reference", ref.RefSitePath(), "/yyz")
ref = NewReference(&doc, nodeElem, "zzy")
ref, _ = NewReference(&doc, nodeElem, "zzy")
assert.Equals(t, "internal relative reference", ref.RefSitePath(), "directory/subdir/zzy")
ref = NewReference(&doc, nodeElem, "zzy/uup.jjr")
ref, _ = NewReference(&doc, nodeElem, "zzy/uup.jjr")
assert.Equals(t, "internal relative reference", ref.RefSitePath(), "directory/subdir/zzy/uup.jjr")
ref = NewReference(&doc, nodeElem, "./zzy/uup.jjr")
ref, _ = NewReference(&doc, nodeElem, "./zzy/uup.jjr")
assert.Equals(t, "internal relative reference", ref.RefSitePath(), "directory/subdir/zzy/uup.jjr")
}

Expand Down
10 changes: 9 additions & 1 deletion htmltest/check-generic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,15 @@ func (hT *HTMLTest) checkGeneric(document *htmldoc.Document, node *html.Node, ke
}

urlStr := htmldoc.GetAttr(node.Attr, key)
ref := htmldoc.NewReference(document, node, urlStr)
ref, err := htmldoc.NewReference(document, node, urlStr)
if err != nil {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Document: document,
Message: fmt.Sprintf("bad reference: %q", err),
})
return
}

// Check attr isn't blank
if urlStr == "" {
Expand Down
21 changes: 19 additions & 2 deletions htmltest/check-img.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package htmltest

import (
"fmt"
"github.com/wjdp/htmltest/htmldoc"
"github.com/wjdp/htmltest/issues"
"golang.org/x/net/html"
Expand All @@ -12,7 +13,15 @@ func (hT *HTMLTest) checkImg(document *htmldoc.Document, node *html.Node) {
[]string{"src", "alt", "usemap"})

// Create reference
ref := htmldoc.NewReference(document, node, attrs["src"])
ref, err := htmldoc.NewReference(document, node, attrs["src"])
if err != nil {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Document: document,
Message: fmt.Sprintf("bad reference: %q", err),
})
return
}

// Check alt present, fail if absent unless asked to ignore
if !htmldoc.AttrPresent(node.Attr, "alt") && !hT.opts.IgnoreAltMissing {
Expand Down Expand Up @@ -61,7 +70,15 @@ func (hT *HTMLTest) checkImg(document *htmldoc.Document, node *html.Node) {

// Check usemap
if htmldoc.AttrPresent(node.Attr, "usemap") {
usemapRef := htmldoc.NewReference(document, node, attrs["usemap"])
usemapRef, err := htmldoc.NewReference(document, node, attrs["usemap"])
if err != nil {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Document: document,
Message: fmt.Sprintf("bad reference: %q", err),
})
return
}

if len(usemapRef.URL.Path) > 0 {
hT.issueStore.AddIssue(issues.Issue{
Expand Down
10 changes: 9 additions & 1 deletion htmltest/check-link.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,15 @@ func (hT *HTMLTest) checkLink(document *htmldoc.Document, node *html.Node) {
}

// Create reference
ref := htmldoc.NewReference(document, node, attrs["href"])
ref, err := htmldoc.NewReference(document, node, attrs["href"])
if err != nil {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Document: document,
Message: fmt.Sprintf("bad reference: %q", err),
})
return
}

// Check for missing href, fail for link nodes
if !htmldoc.AttrPresent(node.Attr, "href") {
Expand Down
16 changes: 13 additions & 3 deletions htmltest/check-meta.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package htmltest

import (
"fmt"
"github.com/wjdp/htmltest/htmldoc"
"github.com/wjdp/htmltest/issues"
"golang.org/x/net/html"
Expand All @@ -27,8 +28,9 @@ func (hT *HTMLTest) checkMetaRefresh(document *htmldoc.Document, node *html.Node

// Define ref from this
var ref *htmldoc.Reference
var err error
if len(contentSplit) == 2 {
if contentSplit[1][0]==34 || contentSplit[1][0]==39 {
if contentSplit[1][0] == 34 || contentSplit[1][0] == 39 {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Message: "url in meta refresh must not start with single or double quote",
Expand All @@ -37,9 +39,17 @@ func (hT *HTMLTest) checkMetaRefresh(document *htmldoc.Document, node *html.Node
return
}

ref = htmldoc.NewReference(document, node, contentSplit[1])
ref, err = htmldoc.NewReference(document, node, contentSplit[1])
} else {
ref = htmldoc.NewReference(document, node, "")
ref, err = htmldoc.NewReference(document, node, "")
}
if err != nil {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Document: document,
Message: fmt.Sprintf("bad reference: %q", err),
})
return
}

// If refresh the content attribute must be set
Expand Down
11 changes: 10 additions & 1 deletion htmltest/check-script.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package htmltest

import (
"fmt"
"github.com/wjdp/htmltest/htmldoc"
"github.com/wjdp/htmltest/issues"
"golang.org/x/net/html"
Expand All @@ -11,7 +12,15 @@ func (hT *HTMLTest) checkScript(document *htmldoc.Document, node *html.Node) {
[]string{"src"})

// Create reference
ref := htmldoc.NewReference(document, node, attrs["src"])
ref, err := htmldoc.NewReference(document, node, attrs["src"])
if err != nil {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Document: document,
Message: fmt.Sprintf("bad reference: %q", err),
})
return
}

// Check src problems
if htmldoc.AttrPresent(node.Attr, "src") && len(attrs["src"]) == 0 {
Expand Down

0 comments on commit 6d236af

Please sign in to comment.