Skip to content

Improve cache flags. Proper id on auth errors #309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 6, 2022
Merged

Improve cache flags. Proper id on auth errors #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bce9a79
Improve cache flags. Proper id on auth errors
ryanmelt Dec 4, 2022
d391f4a
fix spec
ryanmelt Dec 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion openc3-cosmos-cmd-tlm-api/app/controllers/api_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,16 @@ def api
OpenC3::Logger.debug("API headers: #{request_headers}", scope: params[:scope], user: user_info(request.headers['HTTP_AUTHORIZATION']))
status, content_type, body = handle_post(request_data, request_headers)
rescue OpenC3::AuthError => error
id = 1
begin
parsed = JSON.parse(request_data, :allow_nan => true)
id = Integer(parsed['id'])
rescue
OpenC3::Logger.warn("Unable to extract id from JSON-RPC message")
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you ran into this condition somehow?

error_code = OpenC3::JsonRpcError::ErrorCode::AUTH_ERROR
response = OpenC3::JsonRpcErrorResponse.new(
OpenC3::JsonRpcError.new(error_code, error.message, error), request.id
OpenC3::JsonRpcError.new(error_code, error.message, error), id
)
status = 401
content_type = "application/json-rpc"
Expand Down
4 changes: 2 additions & 2 deletions openc3/lib/openc3/utilities/bucket_utilities.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# All changes Copyright 2022, OpenC3, Inc.
# All Rights Reserved
#
# This file may also be used under the terms of a commercial license
# This file may also be used under the terms of a commercial license
# if purchased from OpenC3, Inc.

require 'openc3/utilities/bucket'
Expand Down Expand Up @@ -73,7 +73,7 @@ def self.get_cache_control(filename)
has_version_number = /(-|_|\.)\d+(-|_|\.)\d+(-|_|\.)\d+\./.match(filename)
has_content_hash = /\.[a-f0-9]{20}\./.match(filename)
return nil if has_version_number or has_content_hash
return 'no-cache'
return 'no-cache, max-age=3600'
Copy link
Member

@jmthomas jmthomas Dec 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to look this stuff up: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#request_directives
The no-cache request directive asks caches to validate the response with the origin server before reuse. The max-age=N request directive indicates that the client allows a stored response that is generated on the origin server within N seconds — where N may be any non-negative integer (including 0).

So basically we're saying we allow the server to use a stored response that's less than 1hr old but we always want that response from the server (no local caching). Is that right?

end

def self.compress_file(filename, chunk_size = 50_000_000)
Expand Down
2 changes: 1 addition & 1 deletion openc3/spec/models/tool_model_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ module OpenC3
folder = "DEMO"
name = "DEMO"
dir = File.join(SPEC_DIR, "install")
expect(s3).to receive(:put_object).with(bucket: 'tools', key: "#{name}/index.html", body: anything, cache_control: "no-cache", content_type: "text/html", metadata: nil)
expect(s3).to receive(:put_object).with(bucket: 'tools', key: "#{name}/index.html", body: anything, cache_control: "no-cache, max-age=3600", content_type: "text/html", metadata: nil)

model = ToolModel.new(folder_name: folder, name: name, scope: scope, plugin: 'PLUGIN')
model.create
Expand Down