ArgumentError (comparison of Time with String failed) in stylesheet controller

pfaffman · March 7, 2023, 2:27pm

I have a fairly typical subfolder/reverse proxy setup with Apache as the reverse proxy.

First there were issues with insecure assets, so I forced_https in ENV. Now when retrieving assets like stylesheets/color_definitions_light_7_1_6cfae4de9c47a1b8ed3d5748018236d10ea9107e.css?__ws=site.com

Maybe I’m doing something stupid, but I’m getting this:

ArgumentError (comparison of Time with String failed)
app/controllers/stylesheets_controller.rb:66:in `<='
app/controllers/stylesheets_controller.rb:66:in `show_resource'
app/controllers/stylesheets_controller.rb:19:in `show'
app/controllers/application_controller.rb:414:in `block in with_resolved_locale'
app/controllers/application_controller.rb:414:in `with_resolved_locale'
lib/middleware/omniauth_bypass_middleware.rb:74:in `call'
lib/content_security_policy/middleware.rb:12:in `call'
lib/middleware/anonymous_cache.rb:369:in `call'
config/initializers/100-quiet_logger.rb:20:in `call'
config/initializers/100-silence_logger.rb:29:in `call'
lib/middleware/enforce_hostname.rb:24:in `call'
lib/middleware/request_tracker.rb:228:in `call'

pfaffman · March 7, 2023, 2:40pm

Well, looking at

github.com

discourse/discourse/blob/main/app/controllers/stylesheets_controller.rb#L66


      
          # Security note, safe due to route constraint
          underscore_digest = digest ? "_" + digest : ""
          
          
cache_path = Stylesheet::Manager.cache_fullpath
          location = "#{cache_path}/#{target}#{underscore_digest}#{extension}"
          
          
stylesheet_time = query.pick(:created_at)
          
          
handle_missing_cache(location, target, digest) if !stylesheet_time
          
          
if cache_time && stylesheet_time && stylesheet_time <= cache_time
            return render body: nil, status: 304
          end
          
          
unless File.exist?(location)
            if current = query.pick(source_map ? :source_map : :content)
              FileUtils.mkdir_p(cache_path)
              File.write(location, current)
            else
              raise Discourse::NotFound
            end

It looks like stylesheet_time is a string, so changing that line to

    if cache_time && stylesheet_time && stylesheet_time.to_date <= cache_time

solves the problem. I don’t understand how it’s happening now, as I don’t see any recent commits affecting this.

So here’s how it’s set:

    stylesheet_time = query.pick(:created_at)

It would seem like that :created_at would be a proper date. Could some underlying gem have changed? And how would this not have been caught in tests?

pfaffman · March 7, 2023, 3:04pm

Here’s what seems to fix it:

    if !cache_time.to_date.nil? && !stylesheet_time.to_date.nil? 
      if (stylesheet_time.to_date <= cache_time.to_date)
        return render body: nil, status: 304
      end 
    end

It looks like it’s possible for to_date to turn something that’s truthy into a nil date, I guess.

No. That doesn’t work either. I ended up wrapping the section in a begin/rescue/end.

I’m very confused how this could be affecting only this subfolder/reverse-proxied site.

Maybe it has to do with

      cache_time = request.env["HTTP_IF_MODIFIED_SINCE"]

and maybe that’s not getting passed through from Apache or something–or some invalid date string is coming through.

There used to be a rescue nil on that, but that was 5 years ago.

And I think it’s hard to debug since it is a cache, so to debug I’d need to flush that cache to test if it’s working.

Christoph · March 18, 2023, 10:26pm

I had the some problem (also while using the subfolder setup). After fixing an unrelated mixed content warning by setting the force_https setting to true, the issue went away …