Skip to content

add Virtual DMARC mechanism#8

Open
kyamada-github wants to merge 2 commits intoiij:mainfrom
lepidum:vDMARC
Open

add Virtual DMARC mechanism#8
kyamada-github wants to merge 2 commits intoiij:mainfrom
lepidum:vDMARC

Conversation

@kyamada-github
Copy link
Copy Markdown
Contributor

No description provided.

## DMARC
Dmarc.Verify: true
## Add parameter of Virtual DMARC flag
Dmarc.Virtual: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

形容詞の項目名はちょっと違うかなという感じです。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

関連する #8 (comment) でまとめて対応します.

return DmarcAligner_checkSpfAlignment(self, strict_mode);
} // end function: DmarcAligner_checkImpl

bool
Copy link
Copy Markdown
Member

@takahiko takahiko Nov 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static inline 宣言してください。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

承知致しました.

const char *orgl_domain = PublicSuffix_getOrganizationalDomain(self->publicsuffix, domain);
if (NULL != orgl_domain && InetDomain_equals(orgl_domain, self->orgl_authordomain)) {
self->score = DMARC_SCORE_PASS;
self->score = DMARC_SCORE_BESTGUESSPASS;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vDMARC 使ってないときのスコアも変わっちゃいませんか?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認して,改めて回答させていただきます.

Dmarc.Verify: true
## Add parameter of Virtual DMARC flag
Dmarc.Virtual: true
Dmarc.vDmarcStrictVerification: false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

boolean の設定項目を2つ持たせるより、mode を指定する設定項目を1つ持たせる方がわかりやすくないですか?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vDMARC の ON/OFF と, vDMARC を ON にした場合の検証モード (strict/relax) を個別に指定する形式は,設定が階層化されわかりやすいのではないかと考えたのですが,一つにまとめた方がシンプルで良いでしょうか.以下に一つにまとめた場合と二つの設定項目で指定する場合を並べましたので,もう一度ご確認頂きたく思います.

  • 一つにまとめた場合

Dmarc.vdmarcVerification: strict|relax|none

という指定方法ではどうでしょうか.
strict, relax が指定された場合は,vDMARC ON で, mode はそれぞれ strict, relax,none が指定された場合又は無指定の場合は vDMARC OFF を想定しています.

  • 二つの設定項目で指定する場合

#8 (comment) でのご指摘を踏まえ,

Dmarc.virtualVerification: true
Dmarc.virtualVerificationMode: strict|relax

という指定方法で,
Dmarc.virtualVerificationtrue の場合, vDMARC ON で,mode は Dmarc.virtualVerificationMode で指定されたものとなり,
Dmarc.virtualVerificationfalse の場合は,vDMARC OFF となり Dmarc.virtualVerificationMode の指定は無効,
となることを想定しています.

いかがでしょうか.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

前者はまさにイメージ通りです。前者に一票。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認ありがとうございます.
では,前者の「一つにまとめた場合」のように調整します.

PublicSuffix_getOrganizationalDomain(self->publicsuffix, self->authordomain);
if (NULL != self->orgl_authordomain) {
if (NULL != self->orgl_authordomain
|| !DmarcAligner_isVdmarcModeStrict(self)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

発火する条件が大変読みづらいのでインライン展開してください

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

承知致しました.

bool
DmarcAligner_isVdmarcModeStrict(DmarcAligner *self)
{
return !self->virtual_dmarc || self->is_vdmarc_mode_strict;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

前半部分は関数名以上のことをやってるように見えるのですが、

  • 処理内容にふさわしい関数名にする
  • 処理内容を関数名が説明するものに収める
    のいずれかにしていただけませか?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

承知致しました.調整します.

@kyamada-github
Copy link
Copy Markdown
Contributor Author

すみません,プライベートなGitHubのアカウントで reply してしまいました.
kyamada-github == yamakiyo です.

@kyamada-github kyamada-github force-pushed the vDMARC branch 2 times, most recently from 70385e3 to 2e7666e Compare December 7, 2017 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants