Coverity picked up a potential issue with the previous commit d9f5f1fb7
("Ruby: Handle response field arrays") in that a size_t could wrap
around to SIZE_MAX - 1.
This would happen if we were given an empty array of header values.
Fixes: d9f5f1fb7 ("Ruby: Handle response field arrays")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
@xeron on GitHub reported an issue whereby with a Rails 7.1 application
they were getting the following error
2023/10/22 20:57:28 [error] 56#56 [unit] #8: Ruby: Wrong header entry 'value' from application
2023/10/22 20:57:28 [error] 56#56 [unit] #8: Ruby: Failed to run ruby script
After some back and forth debugging it turns out rack was trying to send
back a header comprised of an array of values. E.g
app = Proc.new do |env|
["200", {
"Content-Type" => "text/plain",
"X-Array-Header" => ["Item-1", "Item-2"],
}, ["Hello World\n"]]
end
run app
It seems this became a possibility in rack v3.0[0]
So along with a header value type of T_STRING we need to also allow
T_ARRAY.
If we get a T_ARRAY we need to build up the header field using the given
values.
E.g
"X-Array-Header" => ["Item-1", "", "Item-3", "Item-4"],
becomes
X-Array-Header: Item-1; ; Item-3; Item-4
[0]: <https://github.com/rack/rack/blob/main/UPGRADE-GUIDE.md?plain=1#L26>
Reported-by: Ivan Larionov <xeron.oskom@gmail.com>
Closes: <https://github.com/nginx/unit/issues/974>
Link: <https://github.com/nginx/unit/pull/998>
Tested-by: Timo Stark <t.stark@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
Ruby applications would fail to start if they were using rack v3
2022/09/28 15:48:46 [alert] 0#80912 [unit] Ruby: Failed to parse rack script
2022/09/28 15:48:46 [notice] 80911#80911 app process 80912 exited with code 1
This was due to a change in the rack API
Rack V2
def self.load_file(path, opts = Server::Options.new)
...
cfgfile.sub!(/^__END__\n.*\Z/m, '')
app = new_from_string cfgfile, path
return app, options
end
Rack V3
def self.load_file(path)
...
return new_from_string(config, path)
end
This patch handles _both_ the above APIs by correctly handling the cases
where we do and don't get an array returned from
nxt_ruby_rack_parse_script().
Closes: <https://github.com/nginx/unit/issues/755>
Tested-by: Andrew Clayton <a.clayton@nginx.com>
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
[ Andrew: Patch by Zhidao, commit message by me with input from Zhidao ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
This is a preparatory patch that renames the 'local' and 'local_length'
members of the nxt_unit_request_t structure to 'local_addr' and
'local_addr_length' in preparation for the adding of 'local_port' and
'local_port_length' members.
Suggested-by: Zhidao HONG <z.hong@f5.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
As was reported[0] by @travisbell on GitHub, if running unit from the
terminal in the foreground when hitting ^C to exit it, the ruby
application processes would segfault if they were using threads.
It's not 100% clear where the actual problem lies, but it _looks_ like
it may be in ruby.
The simplest way to deal with this for now is to just ignore SIGINT in
the ruby application processes. Unit will still receive and handle it,
cleanly shutting everything down.
For people who want to handle SIGINT in their ruby application running
under unit they can still trap SIGINT and it will override the ignore.
[0]: https://github.com/nginx/unit/issues/562#issuecomment-1223229585
Closes: https://github.com/nginx/unit/issues/562
Having the basename of the script pathname was incorrect. While
we don't have something more accurate, the best thing to do is to
have it empty (which should be the right thing most of the time).
This closes#715 issue on GitHub.
The bug was introduced in git commit
0032543fa6
'Ruby: added the Rack environment parameter "SCRIPT_NAME".'.
This feature allows one to specify blocks of code that are called when certain
lifecycle events occur. A user configures a "hooks" property on the app
configuration that points to a script. This script will be evaluated on boot
and should contain blocks of code that will be called on specific events.
An example of configuration:
{
"type": "ruby",
"processes": 2,
"threads": 2,
"user": "vagrant",
"group": "vagrant",
"script": "config.ru",
"hooks": "hooks.rb",
"working_directory": "/home/vagrant/unit/rbhooks",
"environment": {
"GEM_HOME": "/home/vagrant/.ruby"
}
}
An example of a valid "hooks.rb" file follows:
File.write("./hooks.#{Process.pid}", "hooks evaluated")
on_worker_boot do
File.write("./worker_boot.#{Process.pid}", "worker booted")
end
on_thread_boot do
File.write("./thread_boot.#{Process.pid}.#{Thread.current.object_id}",
"thread booted")
end
on_thread_shutdown do
File.write("./thread_shutdown.#{Process.pid}.#{Thread.current.object_id}",
"thread shutdown")
end
on_worker_shutdown do
File.write("./worker_shutdown.#{Process.pid}", "worker shutdown")
end
This closes issue #535 on GitHub.
If an exception was raised with a backtrace of zero length, the
nxt_ruby_exception_log() routine would return without logging the
exception class and message. This commit fixes the issue.
Ruby 3.0 deprecated rb_cData with the intention to remove it in release 3.1.
This commit changes references of rb_cData to rb_cObject. This was done so we
can support distributions that package Ruby 3.0, such as Fedora 34.
We also need to call rb_undef_alloc_func because we're no longer deriving from
rb_cData. This prevents unnecessary allocations.
See:
https://docs.ruby-lang.org/en/3.0.0/doc/extension_rdoc.html
"It is recommended that klass derives from a special class called Data
(rb_cData) but not from Object or other ordinal classes. If it doesn't,
you have to call rb_undef_alloc_func(klass)."
The Ruby interpreter expects an explicit setlocale() call before initialization
to pick up character encodings in the "Encoding" class from the environment.
This closes#531 issue on GitHub.
Ruby threads need to be created with GVL; otherwise, an attempt to access
locked resources may occur, causing a crash.
The issue was occasionally reproduced on Ubuntu 18.04 with Ruby 2.5.1
while running test_ruby_application_threads.
Since the introduction of rootfs feature, some language modules
can't be configured multiple times.
Now the configure generates a separate nxt_<module>_mounts.h for
each module compiled.
The process abstraction has changed to:
setup(task, process)
start(task, process_data)
prefork(task, process, mp)
The prefork() occurs in the main process right before fork.
The file src/nxt_main_process.c is completely free of process
specific logic.
The creation of a process now supports a PROCESS_CREATED state. The
The setup() function of each process can set its state to either
created or ready. If created, a MSG_PROCESS_CREATED is sent to main
process, where external setup can be done (required for rootfs under
container).
The core processes (discovery, controller and router) doesn't need
external setup, then they all proceeds to their start() function
straight away.
In the case of applications, the load of the module happens at the
process setup() time and The module's init() function has changed
to be the start() of the process.
The module API has changed to:
setup(task, process, conf)
start(task, data)
As a direct benefit of the PROCESS_CREATED message, the clone(2) of
processes using pid namespaces now doesn't need to create a pipe
to make the child block until parent setup uid/gid mappings nor it
needs to receive the child pid.
There was a change (ruby/ruby@6c70fed) in Ruby 2.6 that moved
RUBY_DESCRIPTION global constant definition out of Init_version().
Unit initialized Ruby incorrectly, so the constant was not defined.
This closes#330 issue on GitHub.
According to CGI/1.1 RFC 3875:
The server MUST set this variable; if the Script-URI does not include a
query component, the QUERY_STRING MUST be defined as an empty string ("").
Python's PEP 333(3) allows omitting it in WSGI interface; PHP docs force no
requirements; PSGI and Rack specifications require it even if empty.
When nginx proxies requests over FastCGI, it always provides QUERY_STRING.
and some PHP apps have been observed to fail if it is missing (see issue
#201 on GitHub).
A drawback of this change (besides a small overhead) is that there will be
no easy way to tell a missing query string from an empty one (i.e. requests
with or without the "?" character); yet, it's negligible compared to the
possible benefits of wider application compatibility.
This closes#226 issue on GitHub.
Previously, the nxt_router_prepare_msg() function expected server host among
other headers unmodified. It's not true anymore since normalization of the
Host header has been introduced in 77aad2c142a0.
The nxt_unit_split_host() function was removed. It didn't work correctly with
IPv6 literals. Anyway, after 77aad2c142a0 the port splitting is done in router
while Host header processing.
