202196 - wrong protocol for host names containing "gopher"

Status: RESOLVED WONTFIX

(Core :: DOM: Navigation, defect)

Description

[Roland Mainz]

2003-04-09-08-trunk on Solaris/SPARC:

Protocol "guessing" is totally wrong for gopher.
Typing "gopher.quux.org" browses to http://gopher.quux.org instead of
gopher://gopher.quux.org ... ;-((

Steps to reproduce:
1. Open Mozilla/Phoenix
2. type "gopher.quux.org" and hit the ENTER key

Result:
Browser goes to http://gopher.quux.org

Expected result:
Browser goes to gopher://gopher.quux.org

Comment 1

[Boris Zbarsky [:bzbarsky]]

See code at
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDefaultURIFixup.cpp#248

Comment 2

[Roland Mainz]

bzbarsky:
Thanks! :)

----

Taking myself...

Comment 3

[Roland Mainz]

Attachment: Patch for 2003-04-08-08-trunk

Comment 4

[Roland Mainz]

Comment on attachment 120649 [details] [diff] [review]
Patch for 2003-04-08-08-trunk

Requesting r=/sr= ...

Comment 5

[Roland Mainz]

Attachment: Same patch as previous one, just updated the comments... :)

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 120650 [details] [diff] [review]
Same patch as previous one, just updated the comments... :)

>     //   ftp4.no-scheme,com
>     //   no-scheme.com/query?foo=http://www.foo.com
>+    //   gopher.quux.org

use no-scheme.com like the other comments.


>+        if (hostSpec.EqualsIgnoreCase("ftp", 3)) {
>+          uriString.Assign(NS_LITERAL_STRING("ftp://") + uriString);
>+        }
>+        else
>+        {
>+          if (hostSpec.EqualsIgnoreCase("gopher", 6)) {
>+            uriString.Assign(NS_LITERAL_STRING("gopher://") + 

Follow bracing conventions for the file, please:

if ()
{
  // code
}
else if ()
{
  // code
}
else
{
  /code
}

Do we want to check that we have a gopher protocol handler before doing that?

Comment 7

[Boris Zbarsky [:bzbarsky]]

ccing adam so that he's in the loop on this... Adam, would you be OK with us
just adding in the gopher:// without checking whether we have that protocol handler?

Comment 8

[Roland Mainz]

Boris Zbarsky wrote:
> ccing adam so that he's in the loop on this... Adam, would you be OK with us
> just adding in the gopher:// without checking whether we have that protocol 
> handler?

I hope none is considering of removing the gopher protocol or we have another
slashdot article and lots of complaints in bugzilla that "mozilla.org kicks the
roots of the internet with feets" ...

Comment 9

[Roland Mainz]

Attachment: New patch per bz's review comments

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 120656 [details] [diff] [review]
New patch per bz's review comments

> I hope none is considering of removing the gopher protocol

Embeddors most certainly are.  Not only considering, but removing.  This is why
nsNetModule2.cpp is separate from nsNetModule.cpp -- it's the "module of crap
no sane embeddor wants".

I'll sr this, 'cause I think it's fine, but this is really Adam's code and he
should be the r=.

Comment 11

[Adam Lock]

There is nothing wrong with the patch, however I wonder if fixup isn't a waste
of time and bloat for esoteric protocols that no one but a few diehards use any
more.

Comment 12

[Adam Lock]

Changing keyword. This isn't a regression since malformed gopher urls have never
been fixed up by Mozilla.

Comment 13

[benc]

The only other protocol I know that is handled this ways is ftp, bug 184180.

Gopher is pretty broken, both in the client and out on the web (see bug 194220 #1).

I think Adam should make the final call, but I would suggest making a short,
test suite and thinking about how this would work, with things like Internet
Keywords and Domain Guessing.

If you type "gopher" and there is no gopher.default.domain host, then Domain
Guessing would take over and send you to www.gopher.com. This domain name points
to a system w/ a web server and no gopher server. You would want to check that
the URL resolution of "gopher" -> "gopher://gopher" would not trip Domain
Guessing, because the result would be gopher://www.gopher.com rather than the
normal http://www.gopher.com.

So turning this on would "break" that behavior.

If Internet Keywords is turned on (as in Netscape 7), right now, you get a
search page that contains no links to a gopher server, only one reference to
anything related to the gopher protocol. In comparison, searching Netscapes
default IK server for "finger" turns up more web site references to that protocol.

So, in this area, you would be changing the behavior from a search result for
"gopher" (which has little to say about the gopher protocol) to mapping the URL
request to a presumably available gopher server.

To me, it would make more design sense to allow a user to turn on this
hostname->scheme mapping, either by scheme ([ ]ftp, [ ]gopher) or as alternate
protocols (probably assuming they are hierarchical in formation). But this is
probably a lot of thought about less than 1% of what people are doing these
days. These things also take an enormous amount of time to engineer and
implement, as I have experienced from working on changing the way Mozilla
presents Internet Keywords and Domain Guessing.

I hope that people in this bug will use this kind of approach to look at the
larger effects of this change before making it on the trunk.

Comment 14

[Adam Lock]

Comment on attachment 120656 [details] [diff] [review]
New patch per bz's review comments

I am in two minds about the patch. It's obvious it will fixup gopher:
protocols, but just for a few (and probably knowledgable) people who know how
to type the URL properly. This would be at the expense of some random web
surfer trying to browse a site that happens to start with gopher and getting
odd errors or blank directory listings and not knowing why.

So I think on balance I think I will minus the patch and suggest we WONTFIX
this bug.

It's not that the patch is bad, but to fix basically background noise usage of
Mozilla and introduce odd behaviour elsewhere seems a bad idea.

If someone wants to change my mind...

Comment 15

[Roland Mainz]

benc wrote:
> The only other protocol I know that is handled this ways is ftp, bug 184180.
> Gopher is pretty broken, both in the client and out on the web (see bug 194220 
> #1)

I am working on thaht and I am going to push that Gopher will be fully
functional in 1.4beta again.

BTW: The argumentation we should turn off gopher for footprint reasons etc. is
for /dev/null - the gopher support code in Mozilla is small, there are bigger
stuff which could be axe'ed first (there are some nice bugs to save a few MB(!!)
in the distribution tarball) and as long there are 

[snip]
> This domain name points
> to a system w/ a web server and no gopher server. You would want to check that
> the URL resolution of "gopher" -> "gopher://gopher" would not trip Domain
> Guessing, because the result would be gopher://www.gopher.com rather than the
> normal http://www.gopher.com.
>
> so turning this on would "break" that behavior.

In theory (for correctness, consistency etc. etc.) we should remove protocol
guesing for "ftp", too - ftp.com does not run a ftp server either... =:-)

Comment 16

[Roland Mainz]

s/and as long there are /and as long there are users who use it and a maintainer
who fixes the bugs (if noone steps-up I can maintain that code...)/

Comment 17

[Roland Mainz]

Comment on attachment 120656 [details] [diff] [review]
New patch per bz's review comments

Adam Lock wrote:
> This would be at the expense of some random web
> surfer trying to browse a site that happens to start with gopher and getting
> odd errors or blank directory listings and not knowing why

The same can happen for ftp as well.
Quick searching through the DNS cache logs of our site reveals that nearly all
queried entries which have "gopher." in the DNS name have gopher servers
running to (to be correctly: port 70 was open, the script did not test whether
there was really a gopher server running).

Requesting r= again, on demand we can file a bug to add prefs to turn 
hostname-based protocol guessing "on"/"off" per protocol (backend is easy, the
prefs dialog is little bit harder)

Comment 18

[Adam Lock]

> In theory (for correctness, consistency etc. etc.) we should remove protocol
> guesing for "ftp", too - ftp.com does not run a ftp server either... =:-)

I know, however http and ftp are two heavily used protocols, whereas gopher
isn't. On the balance of probabilty someone typing an address starting "ftp"
probably means ftp://, but I don't know if the same would be true for gopher. 

And even if it were, the protocol isn't even there for many embedding
distributions, so fixing it up as gopher:// doesn't even make sense. I guess
some code could first call IsScheme("gopher") to test the protocol exists but
this seems overkill too. 

I just feel putting in fixup code to save a few people typing gopher on front is
bloat we don't need.

Perhaps you are asking for the wrong thing here. I wouldn't be against changing
the fixup API to allow protocols (or anyone else for that matter) to register 
their own supplemental fixup objects.

Comment 19

[Jerry Talkington]

(In reply to comment #18)
> Perhaps you are asking for the wrong thing here. I wouldn't be against changing
> the fixup API to allow protocols (or anyone else for that matter) to register 
> their own supplemental fixup objects.

I've filed bug 233425 to do just that, so you may want to mark this one WONTFIX.

Comment 20

[benc]

Adam:
This was targeted for 1.4b, which was over a year ago. Since then, people
shipped products off 1.4, and now people are lining up to jump to the 1.7 branch.

I think the current patch, which adds support via the same mechanism that fixes
ftp.hostnames should be declined. 

Having these features just isn't very contemporary, many domains, including
mozilla.org, are now mapping ftp.hostnames to http servers. The direction we
should be going is considering removing the ancient support for FTP mapping as well.

Furthermore, gopher is a module that regresses a lot and still lacks basic
testable functionaltiy, like returning errors when the hostname is not found.
I'd like to see gopher be able to pass a general networking smoketest before is
given namespace in the URL bar logic.

From a browser's perspective, I'm all for supporting FTP and Gopher, but not
through hard-coded mechanisms like this. It is the same wrongness as really bad
Internet Keyword implementations and ANY kind of domain guessing, the only
difference is the scope of the audience and problem is much smaller.

If this were to work, it should be modular, so we'd need to "put it back"
properly, modular w/ prefs UI. 

Comment 21

[Andrew Schultz]

==> docshell

Comment 22

[benc]

I think we can mark this wontfix now, because of bug 388195.

Comment 23

[benc]

reset owners.

Comment 24

[benc]

WONTFIX:

If this is going to become an add-on, this functionality would need to be added some how, it couldn't live in the core code (as far as I can tell).