Skip to content

Commit

Permalink
Merge pull request #132 from tomtom5152/fix/127
Browse files Browse the repository at this point in the history
Fix failing test with invalid port by adding popper handling of the error
  • Loading branch information
wjdp committed Dec 15, 2019
2 parents d9b1c7e + 6d236af commit 8fed7ea
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
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
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
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
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
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
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
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 8fed7ea

Please sign in to comment.