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.

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

Normalize: ".twingly.com" -> "www.twingly.com"? #70

Closed
dentarg opened this issue Dec 14, 2015 · 5 comments
Closed

Normalize: ".twingly.com" -> "www.twingly.com"? #70

dentarg opened this issue Dec 14, 2015 · 5 comments

Comments

@dentarg
Copy link
Collaborator

dentarg commented Dec 14, 2015

Found this in my git stash, with the name "dot problem" (yes, it looks kinda old, before rspec...)

I will leave it here so I can clean my stash... :) We should probably check with our .NET version.

$ git stash show -p
diff --git a/test/unit/normalization_test.rb b/test/unit/normalization_test.rb
index 939c52c..7fa6a34 100644
--- a/test/unit/normalization_test.rb
+++ b/test/unit/normalization_test.rb
@@ -83,6 +83,13 @@ class NormalizerTest < Test::Unit::TestCase
       assert_equal "http://www.twingly.com/", result
     end

+    should "add www without dot if host is missing a subdomain but include dot" do
+      url = "http://.twingly.com/"
+      result = @normalizer.normalize_url(url)
+
+      assert_equal "http://www.twingly.com/", result
+    end
+
     should "not add www if the host has a subdomain" do
       url = "http://blog.twingly.com/"
       result = @normalizer.normalize_url(url)

Current master (4d507d9)

$ pry -r ./lib/twingly/url.rb
[1] pry(main)> Twingly::URL.parse("http://.twingly.com/").normalized.to_s
=> "http://.twingly.com/"

Slightly related to #52 perhaps.

@walro
Copy link
Contributor

walro commented Dec 14, 2015

Run-time exception (line 7): Invalid URI: The hostname could not be parsed.

Stack Trace:

[System.UriFormatException: Invalid URI: The hostname could not be parsed.]
  at Program.Main(): line 7

@walro
Copy link
Contributor

walro commented Dec 14, 2015

That's the output from new Uri("http//.twingly.com"); I am thinking we should go with NullURL.

@dentarg
Copy link
Collaborator Author

dentarg commented Dec 14, 2015

Tried http://.twingly.com/ in Google Chrome, the dot is removed

Request URL:http://twingly.com/

@dentarg
Copy link
Collaborator Author

dentarg commented Nov 6, 2016

I am thinking we should go with NullURL

Agree.

This one has solved itself 😄

$ pry
[1] pry(main)> require "twingly/url"
=> true
[2] pry(main)> Twingly::URL::VERSION
=> "5.0.1"
[3] pry(main)> Twingly::URL.parse("http://.twingly.com/")
=> #<Twingly::URL::NullURL:0x007fc4e4dcc520>

We have very similar test cases

    "http://.com.my/",
    "http://.net",

but I will add another one just to be sure, before closing this.

@dentarg
Copy link
Collaborator Author

dentarg commented Nov 6, 2016

No, I will close this first, as wontfix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants