Project

General

Profile

Bug #14795

Should 'net/http' require 'net/http/status' ?

Added by sakuro (Sakuro OZAWA) 4 months ago. Updated 3 months ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-darwin17]
[ruby-core:87300]

Description

net/http/status.rb has been added (#12935) but it is not required from net/http.rb while other net/http/*.rb files are already required.
Is this intentional?

$ irb
>> require 'net/http'
=> true
>> Net::HTTP::STATUS_CODES
:
NameError (uninitialized constant Net::HTTP::STATUS_CODES)
>> require 'net/http/status'
=> true
>> Net::HTTP::STATUS_CODES
=> {100=>"Continue", ..., 511=>"Network Authentication Required"}

History

#1 [ruby-core:87304] Updated by shevegen (Robert A. Heiler) 4 months ago

I think it would make sense, given how important http status codes
are in general (and people who use net/http may also usually deal
with http status codes).

#2 [ruby-core:87305] Updated by normalperson (Eric Wong) 4 months ago

shevegen@gmail.com wrote:

I think it would make sense, given how important http status codes
are in general (and people who use net/http may also usually deal
with http status codes).

I disagree, nothing in net/http client code relies on status
code messages. Loading status messages would needlessly
increase the memory footprint of people who only want an
HTTP client.

Having a code number -> message mapping is useful to servers
(but unfortunately having require "net/http" in
net/http/status would needlessly bloat servers, too).

#3 [ruby-core:87332] Updated by naruse (Yui NARUSE) 4 months ago

net/http itself doesn't need net/http/status.
Therefore at this time it's intentional.
Though I may change it if there's a reasonable use case to require it.

#4 [ruby-core:87333] Updated by tonytonyjan (Wei-Hang Jian) 4 months ago

IMHO, I would suggest use Kernel::autoload, thus we don't need to type net/http/status when we want it, and they will never be loaded into memory until we try to access the constant STATUS_CODES. Isn't it win-win?

#5 [ruby-core:87334] Updated by jeremyevans0 (Jeremy Evans) 4 months ago

tonytonyjan (Wei-Hang Jian) wrote:

IMHO, I would suggest use Kernel::autoload, thus we don't need to type net/http/status when we want it, and they will never be loaded into memory until we try to access the constant STATUS_CODES. Isn't it win-win?

No, it's a trade-off, and a bad trade-off in my opinion. See #5653. Even if the particular implementation issues with autoload are or have been addressed, my main problem with autoload is that libraries that use it are a pain to use in security sensitive code that uses Dir.chroot. Well designed libraries should require all files at load/application startup time, and avoid requiring files during application runtime, and autoload is a hidden runtime require.

#6 [ruby-core:87337] Updated by tonytonyjan (Wei-Hang Jian) 4 months ago

jeremyevans0 (Jeremy Evans) wrote:

tonytonyjan (Wei-Hang Jian) wrote:

IMHO, I would suggest use Kernel::autoload, thus we don't need to type net/http/status when we want it, and they will never be loaded into memory until we try to access the constant STATUS_CODES. Isn't it win-win?

No, it's a trade-off, and a bad trade-off in my opinion. See #5653. Even if the particular implementation issues with autoload are or have been addressed, my main problem with autoload is that libraries that use it are a pain to use in security sensitive code that uses Dir.chroot. Well designed libraries should require all files at load/application startup time, and avoid requiring files during application runtime, and autoload is a hidden runtime require.

Hi Jeremy, thank you for the reference, I didn't know that thread before (It seems like there has not been a conclusion yet :(

By the way, I can see that cgi.rb (https://bugs.ruby-lang.org/projects/ruby-trunk/repository/entry/lib/cgi.rb#L296) is still using autoload so I am not quite sure if autoload is really deprecated for stdlib currently.

#7 [ruby-core:87572] Updated by sakuro (Sakuro OZAWA) 3 months ago

naruse (Yui NARUSE) wrote:

net/http itself doesn't need net/http/status.

Agreed. I'm OK to close this issue.

Also available in: Atom PDF