正規表現間違い探しクイズ その2

正規表現間違い探しクイズシリーズです。

正規表現単体テストを書いている場合でもバグを発見しづらいものです。 そのためレビューの時には注意深く見るようにしています。そんな中見つけた間違いのうち印象的だったものを紹介します。

問題編

仕様

Linkモデルにはurlを持ちます。urlは以下の条件を満たします。

  • 入力必須
  • 長さは256文字以下
  • URLの形式を満たす必要がある
  • プロトコルhttpまたはhttpsのみ許可される
  • ドメインwww.example.comまたはblog.example.comのみ許可される

ソースコード

ソースコードです。 今回はRuby on Railsで実装します。

class Link < ApplicationRecord
  validates :url, presence: true, length: {maximum: 256}
  validate :valid_url

  private

  def valid_url
    return unless url

    if ! url?(url) then
      # URLの形式を満たしていない
      errors.add(:url, 'は有効なURLではありません。')
      return
    end

    if ! %r{\Ahttp(s)?://}.match?(url) then
      # httpまたはhttpsで始まっていない
      errors.add(:url, 'のプロトコルはhttpまたはhttpsのみ指定できます。')
    end

    if ! /((www)|(blog))\.example\.com/.match?(url) then
      # ドメインがwww.example.comまたはblog.example.comでない
      errors.add(:url, 'のドメインはwww.example.comまたはblog.example.comである必要があります。')
    end
  end

  def url?(url)
    # 今回の本題ではないので省略
  end
end

さて、上記ソースコードには明らかな間違いがあります。どのような間違いでしょうか。また、どのように修正するべきでしょうか。(ちなみに正規表現のはじめの\Aは文字列の先頭を指します。^と似ていますが、違いは参考のリンクを参照してください)

テスト

以下のテストはパスしています。

require 'rails_helper'

RSpec.describe Link, type: :model do
  describe '#url' do
    subject { link.errors[:url] }

    let(:link) { Link.new(params) }
    let(:params) { {url: url} }

    before do
      link.valid?
    end

    context '許可されるURLの場合' do
      context 'プロトコルがhttpの場合' do
        context 'ドメインがwww.example.comの場合' do
          let(:url) {'http://www.example.com/path/to/something'}
          it { is_expected.to be_blank}
        end
        context 'ドメインがblog.example.comの場合' do
          let(:url) {'http://blog.example.com/path/to/something'}
          it { is_expected.to be_blank}
        end
      end

      context 'プロトコルがhttpsの場合' do
        context 'ドメインがwww.example.comの場合' do
          let(:url) {'https://www.example.com/path/to/something'}
          it { is_expected.to be_blank}
        end
        context 'ドメインがblog.example.comの場合' do
          let(:url) {'https://blog.example.com/path/to/something'}
          it { is_expected.to be_blank}
        end
      end
    end

    context '許可されないURLの場合' do
      context '許可されないプロトコルの場合' do
        let(:url) {'ftp://www.example.com/path/to/something'}
        it { is_expected.to include('のプロトコルはhttpまたはhttpsのみ指定できます。')}
      end

      context '許可されないドメインの場合' do
        let(:url) {'http://www.hatenablog.com/path/to/something'}
        it { is_expected.to include('のドメインはwww.example.comまたはblog.example.comである必要があります。')}
      end
    end
  end
end

RSpec知らない人だと何書いてあるかわからないかと思いますが、重要な部分は、入力がhttp://www.example.com/path/to/somethingだとエラーが出なくて(be_blank)、入力がftp://www.example.com/path/to/somethingだとのプロトコルはhttpまたはhttpsのみ指定できます。というエラーが出るという部分です。

解答編

少し考えてから解答編を見てください

よいですか?

間違いの部分

((www)|(blog))\.example\.comのマッチする範囲がurl全体なのが間違っています。ドメイン以外の部分にwww.example.comが含まれる場合にもバリデーションが通ってしまいます。

テストを追加してみましょう。

RSpec.describe Link, type: :model do
  describe '#url' do
    subject { link.errors[:url] }

    # 略

    context '許可されないURLの場合' do
      # 略
      
      context '許可されないドメインの場合(パスに許可ドメインを含む)' do
        let(:url) {'http://www.hatenablog.com/path/to/www.example.com/'}
        it { is_expected.to include('のドメインはwww.example.comまたはblog.example.comである必要があります。')}
      end
      
      context '許可されないドメインの場合(クエリに許可ドメインを含む)' do
        let(:url) {'http://www.hatenablog.com/path/to/something?q=www.example.com'}
        it { is_expected.to include('のドメインはwww.example.comまたはblog.example.comである必要があります。')}
      end
    end
  end
end

これを実行すると、以下の理由でテストが失敗します。

expected [] to include "のドメインはwww.example.comまたはblog.example.comである必要があります。"

エラーが空配列になっていますね。

どう直すか

正規表現を修正する

ドメインhttp://またはhttps://に続いているので、正規表現にこれらを追加します。

class Link < ApplicationRecord
  # 略

  def valid_url
    # 略

    if ! %r{\Ahttp(s)?://((www)|(blog))\.example\.com}.match?(url) then  # 変更
      # ドメインがwww.example.comまたはblog.example.comでない
      errors.add(:url, 'のドメインはwww.example.comまたはblog.example.comである必要があります。')
    end
  end
end
URIモジュールを使用する

上記方法はドメインのチェックにスキーマ部分が入っていてあまりメンテナンス性にすぐれているとは言えません。 ドメインwww.example.comまたはblog.example.comのみ許可されているので、ドメインをURLから抜き出してから正規表現でマッチさせます。URLからドメイン部分を抽出するには、URI.parseを使用します。このメソッドでスキーマやホスト(ドメイン)、リクエストパスを取得することができます。また、入力がURLの形式を満たしているかも判定できます。

class Link < ApplicationRecord
  validates :url, presence: true, length: {maximum: 256}
  validate :valid_url

  private

  def valid_url
    return unless url

    begin
      uri = URI.parse(url)
    rescue
      errors.add(:url, 'は有効なURLではありません。')
      return
    end

    if ! %r{\Ahttp(s)?\z}.match?(uri.scheme) then
      # httpまたはhttpsで始まっていない
      errors.add(:url, 'のプロトコルはhttpまたはhttpsのみ指定できます。')
    end

    if ! /\A((www)|(blog))\.example\.com\z/.match?(uri.host) then
      # ドメインがwww.example.comまたはblog.example.comでない
      errors.add(:url, 'のドメインはwww.example.comまたはblog.example.comである必要があります。')
    end
  end
end

これで仕様が満たされ、テストが通るようになりました。

あとがき

これを書いてる時点では、こんな間違い簡単に見つかるだろうと思うのですが、 レビューしている時はなかなか発見できないんですよね。

参考