From a0859a5586ad225429803808ecd59666bd3e2f49 Mon Sep 17 00:00:00 2001 From: takahashim Date: Sun, 16 Aug 2020 14:09:33 +0900 Subject: [PATCH] use h() instead of CGI.escapeHTML() * in current Ruby, use `cgi/escape` first and `cgi/util` as fallback, because `cgi/escape` is fast * remove orignal implementation in ReVIEW::HTMLUtils.escape() --- lib/epubmaker/epubcommon.rb | 52 +++++++++++++++++++++---------------- lib/epubmaker/epubv2.rb | 9 +++---- lib/epubmaker/epubv3.rb | 36 ++++++++++++------------- lib/review/builder.rb | 1 - lib/review/epub2html.rb | 7 ++++- lib/review/epubmaker.rb | 14 +++++----- lib/review/htmlutils.rb | 17 +++++------- lib/review/idgxmlmaker.rb | 7 +++-- lib/review/makerhelper.rb | 11 ++++++++ lib/review/webmaker.rb | 2 +- test/test_htmlutils.rb | 12 --------- 11 files changed, 89 insertions(+), 79 deletions(-) diff --git a/lib/epubmaker/epubcommon.rb b/lib/epubmaker/epubcommon.rb index 971533ce9..5ee2348c9 100644 --- a/lib/epubmaker/epubcommon.rb +++ b/lib/epubmaker/epubcommon.rb @@ -10,7 +10,11 @@ require 'review/i18n' require 'review/template' -require 'cgi' +begin + require 'cgi/escape' +rescue LoadError + require 'cgi/util' +end module EPUBMaker # EPUBCommon is the common class for EPUB producer. @@ -22,6 +26,10 @@ def initialize(producer) @body_ext = nil end + def h(str) + CGI.escapeHTML(str) + end + # Return mimetype content. def mimetype 'application/epub+zip' @@ -59,10 +67,10 @@ def ncx_isbn def ncx_doctitle < - #{CGI.escapeHTML(@producer.config['title'])} + #{h(@producer.config['title'])} - #{@producer.config['aut'].nil? ? '' : CGI.escapeHTML(join_with_separator(@producer.config['aut'], ReVIEW::I18n.t('names_splitter')))} + #{@producer.config['aut'].nil? ? '' : h(join_with_separator(@producer.config['aut'], ReVIEW::I18n.t('names_splitter')))} EOT end @@ -72,7 +80,7 @@ def ncx_navmap(indentarray) - #{CGI.escapeHTML(@producer.config['title'])} + #{h(@producer.config['title'])} @@ -84,7 +92,7 @@ def ncx_navmap(indentarray) s << < - #{CGI.escapeHTML(@producer.res.v('toctitle'))} + #{h(@producer.res.v('toctitle'))} @@ -100,7 +108,7 @@ def ncx_navmap(indentarray) s << < - #{indent[level]}#{CGI.escapeHTML(item.title)} + #{indent[level]}#{h(item.title)} @@ -131,21 +139,21 @@ def cover(type = nil) raise "coverimage #{@producer.config['coverimage']} not found. Abort." unless file @body = <<-EOT
- #{CGI.escapeHTML(@producer.config.name_of('title'))} + #{h(@producer.config.name_of('title'))}
EOT else @body = <<-EOT -

#{CGI.escapeHTML(@producer.config.name_of('title'))}

+

#{h(@producer.config.name_of('title'))}

EOT if @producer.config['subtitle'] @body << <<-EOT -

#{CGI.escapeHTML(@producer.config.name_of('subtitle'))}

+

#{h(@producer.config.name_of('subtitle'))}

EOT end end - @title = CGI.escapeHTML(@producer.config.name_of('title')) + @title = h(@producer.config.name_of('title')) @language = @producer.config['language'] @stylesheets = @producer.config['stylesheet'] tmplfile = if @producer.config['htmlversion'].to_i == 5 @@ -161,7 +169,7 @@ def cover(type = nil) # NOTE: this method is not used yet. # see lib/review/epubmaker.rb#build_titlepage def titlepage - @title = CGI.escapeHTML(@producer.config.name_of('title')) + @title = h(@producer.config.name_of('title')) @body = <#{@title} @@ -169,7 +177,7 @@ def titlepage if @producer.config['subtitle'] @body << <#{CGI.escapeHTML(@producer.config.name_of('subtitle'))} +

#{h(@producer.config.name_of('subtitle'))}

EOT end @@ -179,7 +187,7 @@ def titlepage

-

#{CGI.escapeHTML(join_with_separator(@producer.config.names_of('aut'), ReVIEW::I18n.t('names_splitter')))}

+

#{h(join_with_separator(@producer.config.names_of('aut'), ReVIEW::I18n.t('names_splitter')))}

EOT end @@ -192,7 +200,7 @@ def titlepage

-

#{CGI.escapeHTML(join_with_separator(publisher, ReVIEW::I18n.t('names_splitter')))}

+

#{h(join_with_separator(publisher, ReVIEW::I18n.t('names_splitter')))}

EOT end @@ -209,18 +217,18 @@ def titlepage # Return colophon content. def colophon - @title = CGI.escapeHTML(@producer.res.v('colophontitle')) + @title = h(@producer.res.v('colophontitle')) @body = < EOT if @producer.config['subtitle'].nil? @body << <#{CGI.escapeHTML(@producer.config.name_of('title'))}

+

#{h(@producer.config.name_of('title'))}

EOT else @body << <#{CGI.escapeHTML(@producer.config.name_of('title'))}
#{CGI.escapeHTML(@producer.config.name_of('subtitle'))}

+

#{h(@producer.config.name_of('title'))}
#{h(@producer.config.name_of('subtitle'))}

EOT end @@ -229,7 +237,7 @@ def colophon @body << %Q( \n) @body << @producer.config['colophon_order'].map do |role| if @producer.config[role] - %Q( \n) + %Q( \n) else '' end @@ -238,7 +246,7 @@ def colophon @body << %Q( \n) if @producer.isbn_hyphen @body << %Q(
#{CGI.escapeHTML(@producer.res.v(role))}#{CGI.escapeHTML(join_with_separator(@producer.config.names_of(role), ReVIEW::I18n.t('names_splitter')))}
#{h(@producer.res.v(role))}#{h(join_with_separator(@producer.config.names_of(role), ReVIEW::I18n.t('names_splitter')))}
ISBN#{@producer.isbn_hyphen}
\n) if @producer.config['rights'] && !@producer.config['rights'].empty? - @body << %Q( \n) + @body << %Q( \n) end @body << %Q( \n) @@ -289,9 +297,9 @@ def date_to_s(date) # Return own toc content. def mytoc - @title = CGI.escapeHTML(@producer.res.v('toctitle')) + @title = h(@producer.res.v('toctitle')) - @body = %Q(

#{CGI.escapeHTML(@producer.res.v('toctitle'))}

\n) + @body = %Q(

#{h(@producer.res.v('toctitle'))}

\n) if @producer.config['epubmaker']['flattoc'].nil? @body << hierarchy_ncx('ul') else @@ -385,7 +393,7 @@ def flat_ncx(type, indent = nil) @producer.contents.each do |item| next if !item.notoc.nil? || item.level.nil? || item.file.nil? || item.title.nil? || item.level > @producer.config['toclevel'].to_i is = indent == true ? ' ' * item.level : '' - s << %Q(
  • #{is}#{CGI.escapeHTML(item.title)}
  • \n) + s << %Q(
  • #{is}#{h(item.title)}
  • \n) end s << %Q(\n) diff --git a/lib/epubmaker/epubv2.rb b/lib/epubmaker/epubv2.rb index d7a998ac5..0a1405a51 100644 --- a/lib/epubmaker/epubv2.rb +++ b/lib/epubmaker/epubv2.rb @@ -9,7 +9,6 @@ # require 'epubmaker/epubcommon' -require 'cgi' require 'epubmaker/zip_exporter' module EPUBMaker @@ -37,9 +36,9 @@ def opf_metainfo %w[title language date type format source description relation coverage subject rights].each do |item| next unless @producer.config[item] if @producer.config[item].is_a?(Array) - s << @producer.config.names_of(item).map { |i| %Q( #{CGI.escapeHTML(i)}\n) }.join + s << @producer.config.names_of(item).map { |i| %Q( #{h(i)}\n) }.join else - s << %Q( #{CGI.escapeHTML(@producer.config.name_of(item).to_s)}\n) + s << %Q( #{h(@producer.config.name_of(item).to_s)}\n) end end @@ -54,7 +53,7 @@ def opf_metainfo %w[aut a-adp a-ann a-arr a-art a-asn a-aqt a-aft a-aui a-ant a-bkp a-clb a-cmm a-dsr a-edt a-ill a-lyr a-mdc a-mus a-nrt a-oth a-pht a-prt a-red a-rev a-spn a-ths a-trc a-trl].each do |role| next unless @producer.config[role] @producer.config.names_of(role).each do |v| - s << %Q( #{CGI.escapeHTML(v)}\n) + s << %Q( #{h(v)}\n) end end @@ -62,7 +61,7 @@ def opf_metainfo %w[adp ann arr art asn aqt aft aui ant bkp clb cmm dsr edt ill lyr mdc mus nrt oth pht prt red rev spn ths trc trl].each do |role| next unless @producer.config[role] @producer.config.names_of(role).each do |v| - s << %Q( #{CGI.escapeHTML(v)}\n) + s << %Q( #{h(v)}\n) if role == 'prt' s << %Q( #{v}\n) end diff --git a/lib/epubmaker/epubv3.rb b/lib/epubmaker/epubv3.rb index c6115741d..f0722538e 100644 --- a/lib/epubmaker/epubv3.rb +++ b/lib/epubmaker/epubv3.rb @@ -47,23 +47,23 @@ def opf_metainfo if @producer.config[item].is_a?(Array) @producer.config[item].each_with_index do |v, i| if v.is_a?(Hash) - s << %Q( #{CGI.escapeHTML(v['name'])}\n) + s << %Q( #{h(v['name'])}\n) v.each_pair do |name, val| next if name == 'name' - s << %Q( #{CGI.escapeHTML(val)}\n) + s << %Q( #{h(val)}\n) end else - s << %Q( #{CGI.escapeHTML(v.to_s)}\n) + s << %Q( #{h(v.to_s)}\n) end end elsif @producer.config[item].is_a?(Hash) - s << %Q( #{CGI.escapeHTML(@producer.config[item]['name'])}\n) + s << %Q( #{h(@producer.config[item]['name'])}\n) @producer.config[item].each_pair do |name, val| next if name == 'name' - s << %Q( #{CGI.escapeHTML(val)}\n) + s << %Q( #{h(val)}\n) end else - s << %Q( #{CGI.escapeHTML(@producer.config[item].to_s)}\n) + s << %Q( #{h(@producer.config[item].to_s)}\n) end end @@ -81,14 +81,14 @@ def opf_metainfo next unless @producer.config[role] @producer.config[role].each_with_index do |v, i| if v.is_a?(Hash) - s << %Q( #{CGI.escapeHTML(v['name'])}\n) + s << %Q( #{h(v['name'])}\n) s << %Q( #{role.sub('a-', '')}\n) v.each_pair do |name, val| next if name == 'name' - s << %Q( #{CGI.escapeHTML(val)}\n) + s << %Q( #{h(val)}\n) end else - s << %Q( #{CGI.escapeHTML(v)}\n) + s << %Q( #{h(v)}\n) s << %Q( #{role.sub('a-', '')}\n) end end @@ -99,27 +99,27 @@ def opf_metainfo next unless @producer.config[role] @producer.config[role].each_with_index do |v, i| if v.is_a?(Hash) - s << %Q( #{CGI.escapeHTML(v['name'])}\n) + s << %Q( #{h(v['name'])}\n) s << %Q( #{role}\n) v.each_pair do |name, val| next if name == 'name' - s << %Q( #{CGI.escapeHTML(val)}\n) + s << %Q( #{h(val)}\n) end else - s << %Q( #{CGI.escapeHTML(v)}\n) + s << %Q( #{h(v)}\n) s << %Q( #{role}\n) end if %w[prt pbl].include?(role) if v.is_a?(Hash) - s << %Q( #{CGI.escapeHTML(v['name'])}\n) + s << %Q( #{h(v['name'])}\n) s << %Q( #{role}\n) v.each_pair do |name, val| next if name == 'name' - s << %Q( #{CGI.escapeHTML(val)}\n) + s << %Q( #{h(val)}\n) end else - s << %Q( #{CGI.escapeHTML(v)}\n) + s << %Q( #{h(v)}\n) s << %Q( prt\n) end end @@ -129,7 +129,7 @@ def opf_metainfo ## add custom element if @producer.config['opf_meta'].present? @producer.config['opf_meta'].each do |k, v| - s << %Q( #{CGI.escapeHTML(v)}\n) + s << %Q( #{h(v)}\n) end end @@ -206,11 +206,11 @@ def ncx(indentarray) @body = < -

    #{CGI.escapeHTML(@producer.res.v('toctitle'))}

    +

    #{h(@producer.res.v('toctitle'))}

    #{ncx_main} EOT - @title = CGI.escapeHTML(@producer.res.v('toctitle')) + @title = h(@producer.res.v('toctitle')) @language = @producer.config['language'] @stylesheets = @producer.config['stylesheet'] tmplfile = File.expand_path('./html/layout-html5.html.erb', ReVIEW::Template::TEMPLATE_DIR) diff --git a/lib/review/builder.rb b/lib/review/builder.rb index 5e4c37627..dc22a413a 100644 --- a/lib/review/builder.rb +++ b/lib/review/builder.rb @@ -11,7 +11,6 @@ require 'review/compiler' require 'review/sec_counter' require 'stringio' -require 'cgi' require 'fileutils' require 'tempfile' require 'csv' diff --git a/lib/review/epub2html.rb b/lib/review/epub2html.rb index ed0670193..0689573f8 100644 --- a/lib/review/epub2html.rb +++ b/lib/review/epub2html.rb @@ -8,10 +8,15 @@ require 'zip' require 'rexml/document' -require 'cgi' require 'optparse' require 'review/version' +begin + require 'cgi/escape' +rescue + require 'cgi/util' +end + module ReVIEW class Epub2Html def self.execute(*args) diff --git a/lib/review/epubmaker.rb b/lib/review/epubmaker.rb index dbdfe59d2..8718814de 100644 --- a/lib/review/epubmaker.rb +++ b/lib/review/epubmaker.rb @@ -340,9 +340,9 @@ def build_part(part, basetmpdir, htmlfile) File.open(File.join(basetmpdir, htmlfile), 'w') do |f| @body = '' @body << %Q(
    \n) - @body << %Q(

    #{CGI.escapeHTML(ReVIEW::I18n.t('part', part.number))}

    \n) + @body << %Q(

    #{h(ReVIEW::I18n.t('part', part.number))}

    \n) if part.name.strip.present? - @body << %Q(

    #{CGI.escapeHTML(part.name.strip)}

    \n) + @body << %Q(

    #{h(part.name.strip)}

    \n) end @body << %Q(
    \n) @@ -563,19 +563,19 @@ def copy_frontmatter(basetmpdir) def build_titlepage(basetmpdir, htmlfile) # TODO: should be created via epubcommon - @title = CGI.escapeHTML(@config.name_of('booktitle')) + @title = h(@config.name_of('booktitle')) File.open(File.join(basetmpdir, htmlfile), 'w') do |f| @body = '' @body << %Q(
    \n) - @body << %Q(

    #{CGI.escapeHTML(@config.name_of('booktitle'))}

    \n) + @body << %Q(

    #{h(@config.name_of('booktitle'))}

    \n) if @config['subtitle'] - @body << %Q(

    #{CGI.escapeHTML(@config.name_of('subtitle'))}

    \n) + @body << %Q(

    #{h(@config.name_of('subtitle'))}

    \n) end if @config['aut'] - @body << %Q(

    #{CGI.escapeHTML(@config.names_of('aut').join(ReVIEW::I18n.t('names_splitter')))}

    \n) + @body << %Q(

    #{h(@config.names_of('aut').join(ReVIEW::I18n.t('names_splitter')))}

    \n) end if @config['pbl'] - @body << %Q(

    #{CGI.escapeHTML(@config.names_of('pbl').join(ReVIEW::I18n.t('names_splitter')))}

    \n) + @body << %Q(

    #{h(@config.names_of('pbl').join(ReVIEW::I18n.t('names_splitter')))}

    \n) end @body << '
    ' diff --git a/lib/review/htmlutils.rb b/lib/review/htmlutils.rb index bd76d5673..a6de3771d 100644 --- a/lib/review/htmlutils.rb +++ b/lib/review/htmlutils.rb @@ -7,19 +7,16 @@ # the GNU LGPL, Lesser General Public License version 2.1. # -require 'cgi/util' +begin + require 'cgi/escape' +rescue + require 'cgi/util' +end + module ReVIEW module HTMLUtils - ESC = { - '&' => '&', - '<' => '<', - '>' => '>', - '"' => '"' - } # .freeze - def escape(str) - t = ESC - str.gsub(/[&"<>]/) { |c| t[c] } + CGI.escapeHTML(str) end alias_method :escape_html, :escape # for backward compatibility diff --git a/lib/review/idgxmlmaker.rb b/lib/review/idgxmlmaker.rb index 4e7de2c5a..a1bec2ad0 100644 --- a/lib/review/idgxmlmaker.rb +++ b/lib/review/idgxmlmaker.rb @@ -15,9 +15,12 @@ require 'review/yamlloader' require 'review/idgxmlbuilder' require 'review/version' +require 'review/makerhelper' module ReVIEW class IDGXMLMaker + include MakerHelper + attr_accessor :config, :basedir def initialize @@ -155,9 +158,9 @@ def build_part(part, basetmpdir, xmlfile) end f.puts '' f.print '' - f.print CGI.escapeHTML(title) + f.print h(title) f.print '' end apply_filter(File.join(basetmpdir, xmlfile)) diff --git a/lib/review/makerhelper.rb b/lib/review/makerhelper.rb index 6be95054e..796589527 100644 --- a/lib/review/makerhelper.rb +++ b/lib/review/makerhelper.rb @@ -10,6 +10,12 @@ require 'yaml' require 'shellwords' +begin + require 'cgi/escape' +rescue + require 'cgi/util' +end + module ReVIEW module MakerHelper # Return review/bin directory @@ -18,6 +24,11 @@ def bindir end module_function :bindir + def h(str) + CGI.escapeHTML(str) + end + module_function :h + # Copy image files under from_dir to to_dir recursively # ==== Args # from_dir :: path to the directory which has image files to be copied diff --git a/lib/review/webmaker.rb b/lib/review/webmaker.rb index 2b22d5139..d28387931 100644 --- a/lib/review/webmaker.rb +++ b/lib/review/webmaker.rb @@ -284,7 +284,7 @@ def build_titlepage(basetmpdir, htmlfile) File.open("#{basetmpdir}/#{htmlfile}", 'w') do |f| @body = '' @body << %Q(
    ) - @body << %Q(

    #{CGI.escapeHTML(@config.name_of('booktitle'))}

    ) + @body << %Q(

    #{h(@config.name_of('booktitle'))}

    ) if @config['aut'] @body << %Q(

    #{join_with_separator(@config.names_of('aut'), ReVIEW::I18n.t('names_splitter'))}

    ) end diff --git a/test/test_htmlutils.rb b/test/test_htmlutils.rb index 881b56f2a..4215c1d35 100644 --- a/test/test_htmlutils.rb +++ b/test/test_htmlutils.rb @@ -17,18 +17,6 @@ def test_unescape_html assert_equal '&', unescape('&amp;') end - def test_escape_html_ex - keys = ESC.keys - ESC['.'] = 'X' - ESC.each_pair do |ch, ref| - if keys.include?(ch) - assert_equal ref, escape(ch) - else - assert_equal ch, escape(ch) - end - end - end - def test_strip_html assert_equal 'thisistest.', strip_html('

    thisistest

    .') end