rspec-openapi icon indicating copy to clipboard operation
rspec-openapi copied to clipboard

Problem loading openapi.yml after ruby update to 3.2.7

Open ipepe opened this issue 11 months ago • 12 comments

After we updated Ruby from 3.1.4 to 3.2.7 in our project, we noticed that openapi is failing while generating documentation from tests because there is an error of:

Psych::DisallowedClass:
   Tried to load unspecified class: Date

Relatable stack overflow error https://stackoverflow.com/questions/76825550/tried-to-load-unspecified-class-date-psychdisallowedclass-while-updating-th

We fixed it by monkey patching:

RSpec::OpenAPI::SchemaFile.class_eval do
  def read
    return {} unless File.exist?(@path)

    require 'date'

    RSpec::OpenAPI::KeyTransformer.symbolize(YAML.safe_load(File.read(@path), permitted_classes: [Date])) # this can also parse JSON
  end
end

But I also see that there are tests for ruby 3.3 in https://github.com/exoego/rspec-openapi/blob/master/.github/workflows/test.yml#L26 which is confusing because it would seem that they should not be passing. I can submit a PR with change like in the monkey patch but am I the only one that noticed this error while upgrading Ruby?

ipepe avatar Mar 27 '25 13:03 ipepe

@ipepe Sorry, but I can't repro. Could you share Psych version in your installation for repro?

I will accept monkey patch if users need to use Psych of problematic version for some reason (like, a major library depends on it)

exoego avatar Mar 29 '25 07:03 exoego

My Psych::VERSION is 5.0.1. You can try reproducing this issue with this psych version, if that won't make the bug re-appear, I will then try to create a new Rails app with this issue present.

ipepe avatar Apr 01 '25 08:04 ipepe

@exoego try running env OPENAPI=1 bundle exec rspec in this demo repo: https://github.com/ipepe-oss/demo-yaml-issue. For me it returns:


Randomized with seed 17986

Root
  GET /

An error occurred in an `after(:suite)` hook.
Failure/Error: raise DisallowedClass.new('load', klassname)

Psych::DisallowedClass:
  Tried to load unspecified class: Date
# (eval):2:in `date'

Finished in 0.00966 seconds (files took 0.55108 seconds to load)
1 example, 0 failures, 1 error occurred outside of examples

Randomized with seed 17986

ipepe avatar May 28 '25 22:05 ipepe

@ipepe Hello. Regarding how to reproduce the error, it seems to work fine when the YAML value is a string:

date: "2025-01-02"

On the other hand, if the value is unquoted, it attempts to convert it to a Date, which causes an error:

date: 2025-01-02
Psych::DisallowedClass:
  Tried to load unspecified class: Date

However, I believe that RSpec::OpenAPI generates YAML using the quoted string format, like in the first example. That’s also the case in my own application, and I haven’t encountered any errors.

https://github.com/ipepe-oss/demo-yaml-issue For this reproduction repository, wouldn’t applying the following change resolve the issue?

https://github.com/ipepe-oss/demo-yaml-issue/blob/11ac90fa07a4dec5ba492a3af657cf8ef3c1e89e/docs/openapi.yaml#L24

-                date: 2020-01-02
+                date: '2020-01-02'

rhiroe avatar Jun 09 '25 15:06 rhiroe

I dont know how but it geenrates dates without quotes in my problematic original codebase. But also rspec openapi docs claim I can modify Yaml manually: https://github.com/exoego/rspec-openapi?tab=readme-ov-file#how-can-i-add-information-which-cant-be-generated-from-rspec and this is valid YAML to have unquoted date in the file.

ipepe avatar Jun 09 '25 16:06 ipepe

this is valid YAML to have unquoted date in the file.

It's valid YAML, but it's not valid under the OpenAPI Specification. Without quotes, the value is interpreted as a YAML timestamp, which is a different data type from a string.

rhiroe avatar Jun 10 '25 00:06 rhiroe

this is valid YAML to have unquoted date in the file.

It's valid YAML, but it's not valid under the OpenAPI Specification. Without quotes, the value is interpreted as a YAML timestamp, which is a different data type from a string.

You linked YAML 1.1 specification. The OpenAPI 3.x specification recommends using YAML 1.2. Under YAML 1.2 this is parsed as string and is valid for OpenAPI

ipepe avatar Jun 10 '25 00:06 ipepe

I see. Hmm, that said, since there's no real need to convert values to Date objects when using this library, I'm not sure if it's appropriate to include Date (or Time, etc.) in permitted_classes.

rhiroe avatar Jun 10 '25 01:06 rhiroe

I see. Hmm, that said, since there's no real need to convert values to Date objects when using this library, I'm not sure if it's appropriate to include Date (or Time, etc.) in permitted_classes.

I'm open to any solution that would fix the error from happening

ipepe avatar Jun 10 '25 01:06 ipepe

If I understand correctly, we want to load values like 2020-01-02 as a String, not a Date. To achieve that, I believe we need to make changes to how Psych handles parsing.

It would be ideal if we could handle such values as plain strings without them being automatically converted to Date... https://github.com/ruby/psych/blob/e4c36cf55a302771a3dc9fa060b831fa6f1ae991/lib/psych/scalar_scanner.rb#L63-L68

I think it's best to leave the decision of whether to include Date, Time, or other classes in permitted_classes up to the author.

rhiroe avatar Jun 10 '25 01:06 rhiroe

Thanks for such detailed investigation. I now understand it is a semantic gap between OpenAPI/JSON/YAML (string) vs Psych(Date). According to OpenAPI v3.1 spec and JSON Schema, Date and Time will be typed as string with format or pattern attribute.

I prefer that tools just work by default (less configuration). So, Date and Time values in YAML/JSON should be loaded without hassle at user-side.

Using permitted_classes seems legit to fix this issue. PR welcome

exoego avatar Jun 10 '25 01:06 exoego

@exoego @rhiroe PR is ready: https://github.com/exoego/rspec-openapi/pull/256

ipepe avatar Jun 12 '25 18:06 ipepe