improving code quality of jwt module #266
Conversation
![sourcelevel-bot[bot]](https://avatars.githubusercontent.com/in/597?s=60&v=4){kind=small}
lib/jwt.rb
Outdated
| def verify_claims | ||
| Verify.verify_claims(@payload, @options) | ||
| end | ||
| def options_includes_algo_in_header? |
There was a problem hiding this comment.
Use empty lines between method definitions.
lib/jwt.rb
Outdated
| def verify? | ||
| @verify | ||
| end | ||
| def verify_claims |
There was a problem hiding this comment.
Use empty lines between method definitions.
lib/jwt.rb
Outdated
| def allowed_algorithms(options) | ||
| if options.key?(:algorithm) | ||
| [options[:algorithm]] | ||
| def allowed_algorithms |
There was a problem hiding this comment.
Use empty lines between method definitions.
| @@ -22,42 +22,57 @@ def encode(payload, key, algorithm = 'HS256', header_fields = {}) | |||
| encoder.segments | |||
| end | |||
|
|
|||
| def decode(jwt, key = nil, verify = true, custom_options = {}, &keyfinder) | |||
| def decode(jwt, key = nil, verify = true, options = {}, &keyfinder) | |||
There was a problem hiding this comment.
JWT#decode has boolean parameter 'verify'
ab320012
force-pushed
the
master
branch
2 times, most recently
from
db0be71
to
847c8a3
Compare
lib/jwt/decode.rb
Outdated
| @segments.first(2).join('.') | ||
| end | ||
|
|
||
| def parse_and_decode segment |
There was a problem hiding this comment.
Use def with parentheses when there are parameters.
lib/jwt/decode.rb
Outdated
| def validate_segment_count | ||
| raise(JWT::DecodeError, 'Not enough or too many segments') unless | ||
| (@verify and @segments.length != 3) || | ||
| (@segments.length != 3 or @segments.length != 2) |
lib/jwt/decode.rb
Outdated
| private | ||
| def validate_segment_count | ||
| raise(JWT::DecodeError, 'Not enough or too many segments') unless | ||
| (@verify and @segments.length != 3) || |
lib/jwt/decode.rb
Outdated
| required_num_segments = @verify ? [3] : [2, 3] | ||
| raise(JWT::DecodeError, 'Not enough or too many segments') unless required_num_segments.include? segments.length | ||
| segments | ||
| private |
There was a problem hiding this comment.
Useless private access modifier.
lib/jwt/decode.rb
Outdated
| def validate_segment_count | ||
| raise(JWT::DecodeError, 'Not enough or too many segments') unless | ||
| (@verify and @segments.length != 3) || | ||
| (@segments.length != 3 or @segments.length != 2) |
There was a problem hiding this comment.
Use 4 (not 0) spaces for indenting a condition in an unless statement spanning multiple lines.
lib/jwt/decode.rb
Outdated
| rescue JSON::ParserError | ||
| raise JWT::DecodeError, 'Invalid segment encoding' | ||
| end | ||
|
|
There was a problem hiding this comment.
Extra empty line detected at class body end.
lib/jwt/decode.rb
Outdated
| required_num_segments = @verify ? [3] : [2, 3] | ||
| raise(JWT::DecodeError, 'Not enough or too many segments') unless required_num_segments.include? segments.length | ||
| segments | ||
| private |
There was a problem hiding this comment.
Keep a blank line before and after private.
lib/jwt/decode.rb
Outdated
| private | ||
| def validate_segment_count | ||
| raise(JWT::DecodeError, 'Not enough or too many segments') unless | ||
| (@verify and @segments.length != 3) || |
There was a problem hiding this comment.
JWT::Decode#validate_segment_count calls '@segments.length' 3 times
lib/jwt/decode.rb
Outdated
| (@verify && segment_length != 3) || | ||
| (segment_length != 3 || segment_length != 2) | ||
| end | ||
| def segment_length |
There was a problem hiding this comment.
Use empty lines between method definitions.
|
Ebert has finished reviewing this Pull Request and has found:
You can see more details about this review at https://ebertapp.io/github/jwt/ruby-jwt/pulls/266. |
|
@excpt I am thinking about implementing DecodedToken class that will return from JWT#decode. It will act as array with two elements for backward compatability. It should also probably implement payload and header attributes to access the data directly, as well as an errors method. My issue is not sure how to implement the backwards compatibility for errors. if we return an array from #errors method on DecodedToken instead of throwing errors it will break all existing implementations, let me know. #124 |
|
@ab320012 The introduction of Encode/Decode value objects should happen in a different PR. Thanks for all your effort and time you invest! |
I started looking into the #errors and #value object return for decode and wanted to clean up the logic before implementing