Skip to content

Commit

Permalink
Revert "API-24186 New max dimensions for Benefits Intake API (#12457)" (
Browse files Browse the repository at this point in the history
#12518)

This reverts commit e3ce370.
  • Loading branch information
caseywilliams authored May 1, 2023
1 parent d2211d3 commit f91232b
Show file tree
Hide file tree
Showing 19 changed files with 97 additions and 272 deletions.
4 changes: 2 additions & 2 deletions config/features.yml
Original file line number Diff line number Diff line change
Expand Up @@ -776,9 +776,9 @@ features:
va_view_dependents_access:
actor_type: user
description: Allows us to gate the View/ Modify dependents content in a progressive rollout
vba_documents_larger_page_size_limit:
vba_documents_skip_dimension_check:
actor_type: user
description: Allows Benefits Intake to accept PDFs up to 78x101 inches (instead of the default 21x21)
description: Allows Benefits Intake to accept any size of PDF, skipping the check for page dimensions
yellow_ribbon_mvp_enhancement:
actor_type: user
description: Enhances Yellow Ribbon MVP.
Expand Down
32 changes: 7 additions & 25 deletions lib/central_mail/upload_error.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,20 @@
# frozen_string_literal: true

require 'pdf_utilities/pdf_validator'
require 'central_mail/upload_error'

module CentralMail
class UploadError < StandardError
attr_accessor :code, :detail

DEFAULT_MESSAGE = 'Internal Server Error'

# DOC1xx errors: client errors, invalid submissions
DOC101 = 'Invalid multipart payload'
DOC102 = 'Invalid metadata part'
DOC103 = 'Invalid content part'
DOC104 = 'Upload rejected by upstream system'
DOC105 = 'Invalid or unknown id'
DOC106 = 'Maximum document size exceeded.'
DOC106 = 'Maximum document size exceeded. Limit is 100MB per document'
DOC107 = 'Empty payload'
DOC108 = 'Maximum page size exceeded.'
DOC108 = 'Maximum page size exceeded. Limit is 21 in x 21 in.'

# DOC2xx errors: server errors either local or upstream
# not unambiguously related to submitted content
Expand All @@ -26,30 +23,15 @@ class UploadError < StandardError

STATSD_UPLOAD_FAIL_KEY = 'api.central_mail.upload.fail'

def self.extra_message_text(code, pdf_validator_options)
opts = PDFUtilities::PDFValidator::Validator::DEFAULT_OPTIONS.merge(pdf_validator_options.to_h)

case code.to_s
when 'DOC106'
"Limit is #{PDFUtilities.formatted_file_size(opts[:size_limit_in_bytes])} per document."
when 'DOC108'
"Limit is #{opts[:width_limit_in_inches]} in x #{opts[:height_limit_in_inches]} in."
else
''
end
end

def initialize(message = nil, code: nil, detail: nil, pdf_validator_options: {})
if message.nil? && code.present?
def initialize(message = nil, code: nil, detail: nil)
if message.nil?
begin
message = UploadError.const_get code.to_sym
extra = UploadError.extra_message_text(code, pdf_validator_options)
message += " #{extra}" if extra.present?
message = UploadError.const_get code if code.present?
rescue NameError
message = DEFAULT_MESSAGE
message = 'Internal Server Error'
end
end
super(message || DEFAULT_MESSAGE)
super(message)
@code = code
@detail = detail

Expand Down
9 changes: 7 additions & 2 deletions lib/pdf_info.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,13 @@ def pages
end

def page_size
width, height = self['Page size'].scan(/\d+/).map(&:to_i)
{ width:, height: }
page_size_str = self['Page size']
height = page_size_str.split('x')[0].strip.to_i
width = page_size_str.split('x')[1].strip.split[0].to_i
{
height:,
width:
}
end

def page_size_inches
Expand Down
42 changes: 20 additions & 22 deletions lib/pdf_utilities/pdf_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,6 @@
require 'pdf_info'

module PDFUtilities
def self.formatted_file_size(file_size_in_bytes)
bytes_per_gb = 1_000_000_000
bytes_per_mb = 1_000_000
bytes_per_kb = 1_000

if file_size_in_bytes >= bytes_per_gb
"#{format('%g', (file_size_in_bytes.to_f / bytes_per_gb))} GB"
elsif file_size_in_bytes >= bytes_per_mb
"#{format('%g', (file_size_in_bytes.to_f / bytes_per_mb))} MB"
elsif file_size_in_bytes >= bytes_per_kb
"#{format('%g', (file_size_in_bytes.to_f / bytes_per_kb))} KB"
else
"#{file_size_in_bytes} bytes"
end
end

module PDFValidator
FILE_SIZE_LIMIT_EXCEEDED_MSG = 'Document exceeds the file size limit'
PAGE_SIZE_LIMIT_EXCEEDED_MSG = 'Document exceeds the page size limit'
Expand Down Expand Up @@ -46,10 +30,9 @@ class Validator
DEFAULT_OPTIONS = {
size_limit_in_bytes: 100_000_000, # 100 MB
check_page_dimensions: true,
check_encryption: true,
# Height/width limits are ignored if the check_page_dimensions option is false.
width_limit_in_inches: 21,
height_limit_in_inches: 21
width_limit_in_inches: 21, # Ignored if check_page_dimensions: false
height_limit_in_inches: 21, # Ignored if check_page_dimensions: false
check_encryption: true
}.freeze

attr_accessor :result, :pdf_metadata
Expand Down Expand Up @@ -77,8 +60,7 @@ def validate
def check_file_size
size_limit = @options[:size_limit_in_bytes].to_i
if File.size(@file) > size_limit
message = "#{FILE_SIZE_LIMIT_EXCEEDED_MSG} of #{PDFUtilities.formatted_file_size(size_limit)}"
@result.add_error(message)
@result.add_error("#{FILE_SIZE_LIMIT_EXCEEDED_MSG} of #{formatted_file_size(size_limit)}")
end
end

Expand Down Expand Up @@ -107,6 +89,22 @@ def check_page_size
end
end
end

def formatted_file_size(file_size_in_bytes)
bytes_per_gb = 1_000_000_000
bytes_per_mb = 1_000_000
bytes_per_kb = 1_000

if file_size_in_bytes >= bytes_per_gb
"#{format('%g', (file_size_in_bytes.to_f / bytes_per_gb))} GB"
elsif file_size_in_bytes >= bytes_per_mb
"#{format('%g', (file_size_in_bytes.to_f / bytes_per_mb))} MB"
elsif file_size_in_bytes >= bytes_per_kb
"#{format('%g', (file_size_in_bytes.to_f / bytes_per_kb))} KB"
else
"#{file_size_in_bytes} bytes"
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ def download_and_process
validate_metadata(parts[META_PART_NAME], submission_version: @upload.metadata['version'].to_i)
metadata = perfect_metadata(@upload, parts, timestamp)

pdf_validator_options = VBADocuments::DocumentRequestValidator.pdf_validator_options
pdf_validator_options[:check_page_dimensions] = false if metadata['skipDimensionCheck'].present?
pdf_validator_options = metadata['skipDimensionCheck'] ? { check_page_dimensions: false } : {}
validate_documents(parts, pdf_validator_options)

response = submit(metadata, parts)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,13 @@ class DocumentRequestValidator
MAX_FILE_SIZE_IN_BYTES = 100_000_000 # 100 MB
DOCUMENT_NOT_PROVIDED_MSG = 'Document was not provided'
DOCUMENT_NOT_A_PDF_MSG = 'Document is not a PDF'
FILE_SIZE_LIMIT_EXCEEDED_MSG = \
"Document exceeds the file size limit of #{MAX_FILE_SIZE_IN_BYTES / 1_000_000} MB".freeze
FILE_SIZE_LIMIT_EXCEEDED_MSG = 'Document exceeds the file size limit of 100 MB'
DOCUMENT_FAILED_VALIDATION_MSG = 'Document failed validation'

attr_accessor :result

def self.pdf_validator_options
larger_limit = Flipper.enabled?(:vba_documents_larger_page_size_limit)
# Skip the check for owner/permissions password - only a user password invalidates the PDF
PDF_VALIDATOR_OPTIONS = { check_encryption: false }.freeze

{
check_encryption: false, # Owner passwords are allowed, user passwords are not
size_limit_in_bytes: MAX_FILE_SIZE_IN_BYTES,
width_limit_in_inches: larger_limit ? 78 : 21,
height_limit_in_inches: larger_limit ? 101 : 21
}
end
attr_accessor :result

def initialize(request)
@request = request
Expand Down Expand Up @@ -78,7 +69,8 @@ def validate_body
Tempfile.create("vba-documents-validate-#{SecureRandom.hex}.pdf", binmode: true) do |tempfile|
tempfile << @request.body.read
tempfile.rewind
validator = PDFValidator::Validator.new(tempfile, DocumentRequestValidator.pdf_validator_options)
options = Flipper.enabled?(:vba_documents_skip_dimension_check) ? { check_page_dimensions: false } : {}
validator = PDFValidator::Validator.new(tempfile, PDF_VALIDATOR_OPTIONS.merge(options))
result = validator.validate

unless result.valid_pdf?
Expand Down
11 changes: 3 additions & 8 deletions modules/vba_documents/lib/vba_documents/pdf_inspector.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
# frozen_string_literal: true

require 'central_mail/utilities'
require 'pdf_info'
require 'pdf_utilities/pdf_validator'
require 'vba_documents/document_request_validator'
require 'vba_documents/multipart_parser'
require 'pdf_info'
require 'central_mail/utilities'

module VBADocuments
class PDFInspector
Expand Down Expand Up @@ -84,16 +82,13 @@ def add_line_of_business(data, parts_metadata)
def pdf_metadata(pdf)
metadata = PdfInfo::Metadata.read(pdf)
dimensions = metadata.page_size_inches
max_width, max_height = VBADocuments::DocumentRequestValidator.pdf_validator_options.values_at(
:width_limit_in_inches, :height_limit_in_inches
)

{
page_count: metadata.pages,
dimensions: {
height: dimensions[:height].round(2),
width: dimensions[:width].round(2),
oversized_pdf: dimensions[:height] > max_height || dimensions[:width] > max_width
oversized_pdf: dimensions[:height] > 21 || dimensions[:width] > 21
},
sha256_checksum: Digest::SHA256.file(pdf).hexdigest
}
Expand Down
20 changes: 14 additions & 6 deletions modules/vba_documents/lib/vba_documents/upload_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def validate_metadata(metadata_input, submission_version:)
raise VBADocuments::UploadError.new(code: 'DOC102', detail: 'Invalid JSON object')
end

def validate_documents(parts, pdf_validator_options = VBADocuments::DocumentRequestValidator.pdf_validator_options)
def validate_documents(parts, pdf_validator_options = {})
# Validate 'content' document
validate_document(parts[DOC_PART_NAME], DOC_PART_NAME, pdf_validator_options)

Expand Down Expand Up @@ -112,24 +112,32 @@ def validate_line_of_business(lob, submission_version)
end
end

DEFAULT_PDF_VALIDATOR_OPTIONS = {
check_encryption: false # Owner passwords are allowed, user passwords are not
}.freeze

def validate_document(file_path, part_name, pdf_validator_options = {})
result = PDFValidator::Validator.new(file_path, pdf_validator_options).validate
options = DEFAULT_PDF_VALIDATOR_OPTIONS.merge(pdf_validator_options)
options.merge!({ check_page_dimensions: false }) if Flipper.enabled?(:vba_documents_skip_dimension_check)

result = PDFValidator::Validator.new(file_path, options).validate

unless result.valid_pdf?
errors = result.errors

if errors.grep(/#{PDFValidator::FILE_SIZE_LIMIT_EXCEEDED_MSG}/).any?
raise VBADocuments::UploadError.new(code: 'DOC106', pdf_validator_options:)
raise VBADocuments::UploadError.new(code: 'DOC106',
detail: 'Maximum document size exceeded. Limit is 100MB per document')
end

if errors.grep(/#{PDFValidator::USER_PASSWORD_MSG}|#{PDFValidator::INVALID_PDF_MSG}/).any?
raise VBADocuments::UploadError.new(code: 'DOC103',
detail: "Invalid PDF content, part #{part_name}",
pdf_validator_options:)
detail: "Invalid PDF content, part #{part_name}")
end

if errors.grep(/#{PDFValidator::PAGE_SIZE_LIMIT_EXCEEDED_MSG}/).any?
raise VBADocuments::UploadError.new(code: 'DOC108', pdf_validator_options:)
raise VBADocuments::UploadError.new(code: 'DOC108',
detail: VBADocuments::UploadError::DOC108)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions modules/vba_documents/spec/factories/upload_submissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
source: nil, doc_type: 'Unknown', total_documents: 2, total_pages: 2,
content: {
page_count: 1,
dimensions: { height: 11.0, width: 8.5, oversized_pdf: false },
dimensions: { height: 8.5, width: 11.0, oversized_pdf: false },
sha256_checksum: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855',
attachments: [
{
page_count: 1,
dimensions: { height: 11.0, width: 8.5, oversized_pdf: false },
dimensions: { height: 8.5, width: 11.0, oversized_pdf: false },
sha256_checksum: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'
}
]
Expand Down
Binary file removed modules/vba_documents/spec/fixtures/10x102.pdf
Binary file not shown.
Binary file removed modules/vba_documents/spec/fixtures/79x10.pdf
Binary file not shown.
41 changes: 10 additions & 31 deletions modules/vba_documents/spec/lib/document_request_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,42 +25,21 @@
describe 'given a document with large pages' do
let(:fixture_name) { '18x22.pdf' }

context 'when vba_documents_larger_page_size_limit flag is off' do
before { Flipper.disable(:vba_documents_larger_page_size_limit) }
context 'when vba_documents_skip_dimension_check flag is off' do
before { Flipper.disable(:vba_documents_skip_dimension_check) }

describe 'with large PDF' do
it 'errors' do
expect(result[:errors].length).to eq(1)
expect(result[:errors].first[:status]).to eq('422')
end
end

describe 'with extra large PDF' do
let(:fixture_name) { '10x102.pdf' }

it 'errors' do
expect(result[:errors].length).to eq(1)
expect(result[:errors].first[:status]).to eq('422')
end
it 'considers the PDF invalid' do
errors = result[:errors]
expect(errors.length).to eq(1)
expect(errors.first[:status]).to eq('422')
end
end

context 'when vba_documents_larger_page_size_limit flag is on' do
before { Flipper.enable(:vba_documents_larger_page_size_limit) }

describe 'with large PDF' do
it 'considers the PDF valid' do
expect(result.dig(:data, :attributes, :status)).to eq('valid')
end
end

describe 'with extra large PDF' do
let(:fixture_name) { '10x102.pdf' }
context 'when vba_documents_skip_dimension_check flag is on' do
before { Flipper.enable(:vba_documents_skip_dimension_check) }

it 'errors' do
expect(result[:errors].length).to eq(1)
expect(result[:errors].first[:status]).to eq('422')
end
it 'considers the PDF valid' do
expect(result.dig(:data, :attributes, :status)).to eq('valid')
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions modules/vba_documents/spec/lib/pdf_inspector_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
subject { @inspector.pdf_data }

let(:sha256_char_length) { 64 }
let(:page_height) { 11.0 }
let(:page_width) { 8.5 }
let(:page_height) { 8.5 }
let(:page_width) { 11.0 }

it 'returns a hash' do
expect(subject).to be_a(Hash)
Expand Down
Loading

0 comments on commit f91232b

Please sign in to comment.