feat: gmo gemの更新#9
Conversation
NKF sometimes misjudges an encoding. For example `カソウシテン` isn't
judged correctly.
```ruby
NKF.guess('カソウシテン'.encode("Windows-31J"))
=> #<Encoding:EUC-JP>
```
This causes misconverting. The previous word converts like this.
```ruby
NKF.nkf('-w', 'カソウシテン'.encode("Windows-31J"))
=> "郷骸竹"
```
The specifying input encoding prevent this.
```ruby
NKF.nkf('-S -w', 'カソウシテン'.encode("Windows-31J"))
=> "カソウシテン"
```
Fixes t-k#33.
Specify an input encoding to `NKF` to avoid misjudgement
Fix spec
Add Ruby 3.2 to GitHub Actions test matrix
Remove workaround for old Ruby versions
Paypal決済対応
銀行振込(バーチャル口座 あおぞら)の追加
Auth_Codeが導入され、必須でなくなったため。
動作確認がとれなかったため、削除します。
送金サービス メール送金指示の修正
送金サービス リンクタイプURL取得を追加
This commit adds Ruby 3.3 to the test matrix to ensure to work the gem with it.
CI against Ruby 3.3
There was a problem hiding this comment.
Code Review
This pull request implements support for 3D Secure 2.0 and GANB (Aozora Bank) virtual accounts, adds nkf as a dependency for encoding management, and updates the core API to support JSON responses. The review feedback identifies several critical improvements for the response processing logic, including the need to handle nil results during JSON parsing and the importance of restricting encoding conversions and URL reassembly to non-JSON paths to prevent data corruption or runtime errors. Additionally, using more robust JSON generation methods in the HTTP service layer is recommended to ensure consistency and reliability.
| parsed_result = ::JSON.parse(result.body) | ||
| response = parsed_result.is_a?(Array) ? parsed_result[0] : parsed_result |
There was a problem hiding this comment.
The JSON parsing logic can result in response being nil if the API returns an empty array [] or null. This will cause a NoMethodError in subsequent lines (e.g., line 54 or 59) when attempting to access keys on a nil object. It is safer to ensure response defaults to an empty hash.
parsed_result = ::JSON.parse(result.body)
response = (parsed_result.is_a?(Array) ? parsed_result[0] : parsed_result) || {}| end | ||
| # converting to UTF-8 | ||
| body = response = Hash[response.map { |k,v| [k, NKF.nkf('-w',v)] }] | ||
| body = response = Hash[response.map { |k,v| [k, NKF.nkf('-S -w',v)] }] |
There was a problem hiding this comment.
Applying NKF.nkf with the -S (Shift_JIS) flag to all response values is problematic for JSON responses. First, NKF.nkf expects a string and will raise a TypeError if the JSON contains non-string values (like integers for amounts). Second, forcing Shift_JIS interpretation on a JSON response that is already UTF-8 (as indicated by the charset=UTF-8 header in the VCR cassettes) will lead to data corruption. Encoding conversion should be restricted to non-JSON responses which use Shift_JIS/Windows-31J.
unless /\.json\Z/ =~ path
response = Hash[response.map { |k,v| [k, NKF.nkf('-S -w', v)] }]
end
body = response| if response['RedirectUrl'] && response['RedirectUrl'] != '' && response['t'] && response['t'] != '' && response.keys.index('RedirectUrl') + 1 == response.keys.index('t') | ||
| response['RedirectUrl'] = response['RedirectUrl'] + '&t=' + response['t'] | ||
| response.delete('t') | ||
| end |
There was a problem hiding this comment.
This logic for reassembling RedirectUrl is brittle as it relies on the exact order of keys in the hash and only handles the t parameter. More importantly, this transformation is only necessary for responses parsed from query strings where the URL was incorrectly split by the & delimiter; it should not be applied to JSON responses where the URL is already correctly parsed as a single string. Applying it to JSON might incorrectly modify the URL if a top-level t key happens to exist in the response.
if path !~ /\.json\Z/ && response['RedirectUrl'] && response['t'] && response.keys.index('RedirectUrl') + 1 == response.keys.index('t')
response['RedirectUrl'] = "#{response['RedirectUrl']}&t=#{response['t']}"
response.delete('t')
end| h.post(path, encode_params(args)) | ||
| if /\.json\Z/ =~ path | ||
| headers = { "Content-Type" => "application/json" } | ||
| h.post(path, args.to_json, headers) |
There was a problem hiding this comment.
gmo gemの最新化
5.0.4 → 5.0.9