From 4761235f6944d1627346ca835a192c1ed32f745e Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Thu, 31 Aug 2017 20:11:22 +0200 Subject: Validate unescaped `%` chars in PO files --- lib/gitlab/i18n/po_linter.rb | 21 ++++++-- lib/gitlab/i18n/translation_entry.rb | 18 +++++++ spec/fixtures/unescaped_chars.po | 21 ++++++++ spec/lib/gitlab/i18n/po_linter_spec.rb | 14 +++++- spec/lib/gitlab/i18n/translation_entry_spec.rb | 70 ++++++++++++++++++++++++++ 5 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 spec/fixtures/unescaped_chars.po diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb index c3b1fe582af..2e02787a4f4 100644 --- a/lib/gitlab/i18n/po_linter.rb +++ b/lib/gitlab/i18n/po_linter.rb @@ -63,10 +63,25 @@ module Gitlab validate_variables(errors, entry) validate_newlines(errors, entry) validate_number_of_plurals(errors, entry) + validate_unescaped_chars(errors, entry) errors end + def validate_unescaped_chars(errors, entry) + if entry.msgid_contains_unescaped_chars? + errors << 'contains unescaped `%`, escape it using `%%`' + end + + if entry.plural_id_contains_unescaped_chars? + errors << 'plural id contains unescaped `%`, escape it using `%%`' + end + + if entry.translations_contain_unescaped_chars? + errors << 'translation contains unescaped `%`, escape it using `%%`' + end + end + def validate_number_of_plurals(errors, entry) return unless metadata_entry&.expected_plurals return unless entry.translated? @@ -79,15 +94,15 @@ module Gitlab def validate_newlines(errors, entry) if entry.msgid_contains_newlines? - errors << "is defined over multiple lines, this breaks some tooling." + errors << 'is defined over multiple lines, this breaks some tooling.' end if entry.plural_id_contains_newlines? - errors << "plural is defined over multiple lines, this breaks some tooling." + errors << 'plural is defined over multiple lines, this breaks some tooling.' end if entry.translations_contain_newlines? - errors << "has translations defined over multiple lines, this breaks some tooling." + errors << 'has translations defined over multiple lines, this breaks some tooling.' end end diff --git a/lib/gitlab/i18n/translation_entry.rb b/lib/gitlab/i18n/translation_entry.rb index 8d4fec0decd..e6c95afca7e 100644 --- a/lib/gitlab/i18n/translation_entry.rb +++ b/lib/gitlab/i18n/translation_entry.rb @@ -1,6 +1,8 @@ module Gitlab module I18n class TranslationEntry + PERCENT_REGEX = /(?:^|[^%])%(?!{\w*}|[a-z%])/.freeze + attr_reader :nplurals, :entry_data def initialize(entry_data, nplurals) @@ -64,6 +66,22 @@ module Gitlab all_translations.any? { |translation| translation.is_a?(Array) } end + def msgid_contains_unescaped_chars? + contains_unescaped_chars?(msgid) + end + + def plural_id_contains_unescaped_chars? + contains_unescaped_chars?(plural_id) + end + + def translations_contain_unescaped_chars? + all_translations.any? { |translation| contains_unescaped_chars?(translation) } + end + + def contains_unescaped_chars?(string) + string =~ PERCENT_REGEX + end + private def translation_keys diff --git a/spec/fixtures/unescaped_chars.po b/spec/fixtures/unescaped_chars.po new file mode 100644 index 00000000000..fbafe523fb3 --- /dev/null +++ b/spec/fixtures/unescaped_chars.po @@ -0,0 +1,21 @@ +# Spanish translations for gitlab package. +# Copyright (C) 2017 THE PACKAGE'S COPYRIGHT HOLDER +# This file is distributed under the same license as the gitlab package. +# FIRST AUTHOR , 2017. +# +msgid "" +msgstr "" +"Project-Id-Version: gitlab 1.0.0\n" +"Report-Msgid-Bugs-To: \n" +"PO-Revision-Date: 2017-07-13 12:10-0500\n" +"Language-Team: Spanish\n" +"Language: es\n" +"MIME-Version: 1.0\n" +"Content-Type: text/plain; charset=UTF-8\n" +"Content-Transfer-Encoding: 8bit\n" +"Plural-Forms: nplurals=2; plural=n != 1;\n" +"Last-Translator: Bob Van Landuyt \n" +"X-Generator: Poedit 2.0.2\n" + +msgid "You are going to transfer %{project_name_with_namespace} to another owner. Are you ABSOLUTELY sure?" +msgstr "將要把 %{project_name_with_namespace} 的所有權轉移給另一個人。真的「100%確定」要這麼做嗎？" diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb index bd31f0c3871..cd5c2b99751 100644 --- a/spec/lib/gitlab/i18n/po_linter_spec.rb +++ b/spec/lib/gitlab/i18n/po_linter_spec.rb @@ -110,6 +110,17 @@ describe Gitlab::I18n::PoLinter do is_expected.not_to be_empty end end + + context 'with unescaped chars' do + let(:po_path) { 'spec/fixtures/unescaped_chars.po' } + + it 'contains an error' do + message_id = 'You are going to transfer %{project_name_with_namespace} to another owner. Are you ABSOLUTELY sure?' + expected_error = 'translation contains unescaped `%`, escape it using `%%`' + + expect(errors[message_id]).to include(expected_error) + end + end end describe '#parse_po' do @@ -157,13 +168,14 @@ describe Gitlab::I18n::PoLinter do end describe '#validate_entry' do - it 'validates the flags, variable usage, and newlines' do + it 'validates the flags, variable usage, newlines, and unescaped chars' do fake_entry = double expect(linter).to receive(:validate_flags).with([], fake_entry) expect(linter).to receive(:validate_variables).with([], fake_entry) expect(linter).to receive(:validate_newlines).with([], fake_entry) expect(linter).to receive(:validate_number_of_plurals).with([], fake_entry) + expect(linter).to receive(:validate_unescaped_chars).with([], fake_entry) linter.validate_entry(fake_entry) end diff --git a/spec/lib/gitlab/i18n/translation_entry_spec.rb b/spec/lib/gitlab/i18n/translation_entry_spec.rb index d2d3ec03c6d..f68bc8feff9 100644 --- a/spec/lib/gitlab/i18n/translation_entry_spec.rb +++ b/spec/lib/gitlab/i18n/translation_entry_spec.rb @@ -130,4 +130,74 @@ describe Gitlab::I18n::TranslationEntry do expect(entry.translations_contain_newlines?).to be_truthy end end + + describe '#contains_unescaped_chars' do + let(:data) { { msgid: '' } } + let(:entry) { described_class.new(data, 2) } + it 'is true when the msgid is an array' do + string = '「100%確定」' + + expect(entry.contains_unescaped_chars?(string)).to be_truthy + end + + it 'is false when the `%` char is escaped' do + string = '「100%%確定」' + + expect(entry.contains_unescaped_chars?(string)).to be_falsy + end + + it 'is false when using an unnamed variable' do + string = '「100%d確定」' + + expect(entry.contains_unescaped_chars?(string)).to be_falsy + end + + it 'is false when using a named variable' do + string = '「100%{named}確定」' + + expect(entry.contains_unescaped_chars?(string)).to be_falsy + end + + it 'is true when an unnamed variable is not closed' do + string = '「100%{named確定」' + + expect(entry.contains_unescaped_chars?(string)).to be_truthy + end + + it 'is true when the string starts with a `%`' do + string = '%10' + + expect(entry.contains_unescaped_chars?(string)).to be_truthy + end + end + + describe '#msgid_contains_unescaped_chars' do + it 'is true when the msgid contains a `%`' do + data = { msgid: '「100%確定」' } + entry = described_class.new(data, 2) + + expect(entry).to receive(:contains_unescaped_chars?).and_call_original + expect(entry.msgid_contains_unescaped_chars?).to be_truthy + end + end + + describe '#plural_id_contains_unescaped_chars' do + it 'is true when the plural msgid contains a `%`' do + data = { msgid_plural: '「100%確定」' } + entry = described_class.new(data, 2) + + expect(entry).to receive(:contains_unescaped_chars?).and_call_original + expect(entry.plural_id_contains_unescaped_chars?).to be_truthy + end + end + + describe '#translations_contain_unescaped_chars' do + it 'is true when the translation contains a `%`' do + data = { msgstr: '「100%確定」' } + entry = described_class.new(data, 2) + + expect(entry).to receive(:contains_unescaped_chars?).and_call_original + expect(entry.translations_contain_unescaped_chars?).to be_truthy + end + end end -- cgit v1.2.1