From 756f97ede1ba53c52eddb02e6020ede1b896cebd Mon Sep 17 00:00:00 2001 From: Artem Baskal <a.baskal@adguard.com> Date: Tue, 22 Sep 2020 15:04:17 +0300 Subject: [PATCH] + upstream: Allow entering comments to the Upstreams box Close #2083 Squashed commit of the following: commit 113ad3c4ae2ca184b3945dcaa357b57303ee5fd6 Merge: 4ca1f005 bf23aa4d Author: ArtemBaskal <a.baskal@adguard.com> Date: Tue Sep 22 14:41:30 2020 +0300 Merge branch 'master' into feature/2083 commit 4ca1f0056708eb23bb751587a0ec284508f35edf Author: ArtemBaskal <a.baskal@adguard.com> Date: Tue Sep 22 13:14:31 2020 +0300 Simplify filterOutComments, use assert in tests commit bba03568aa979300e0534a2bd2f03086e25b3f87 Author: ArtemBaskal <a.baskal@adguard.com> Date: Tue Sep 22 12:05:00 2020 +0300 Add ValidateUpstreams test cases commit 181de508cf266e3a47058f2b7e1b4b4accbab827 Author: ArtemBaskal <a.baskal@adguard.com> Date: Tue Sep 22 11:47:03 2020 +0300 Refactor testUpstream commit 19c189cf7b64f4d252428dec5a608595c8e4cbc7 Author: ArtemBaskal <a.baskal@adguard.com> Date: Tue Sep 22 10:38:37 2020 +0300 Move functions to utils commit 003937e90e0ff02e696d45c21045a27a49cd0202 Author: ArtemBaskal <a.baskal@adguard.com> Date: Mon Sep 21 19:00:49 2020 +0300 Review changes commit b26bf64d8cef0266f33bce51c5bad324c74bb6da Author: ArtemBaskal <a.baskal@adguard.com> Date: Mon Sep 21 17:58:01 2020 +0300 + upstream: Filter out the upstream comments commit 920975d2ba14fade07282cdb5c72a699c8083463 Author: ArtemBaskal <a.baskal@adguard.com> Date: Mon Sep 21 17:51:00 2020 +0300 Trim upstreams, extract comment token commit a9958eb305ed9af10de68ef3bffe63f216805efe Author: ArtemBaskal <a.baskal@adguard.com> Date: Mon Sep 21 10:34:11 2020 +0300 Fix markup styles commit 6efa41d944c7b09454a4011d2c9ea52b5ce91bbf Author: ArtemBaskal <a.baskal@adguard.com> Date: Fri Sep 18 17:59:57 2020 +0300 Fix upstream form positioning commit 6eb12158d0bca49d4b41eb65a3ebed44eafbe486 Author: ArtemBaskal <a.baskal@adguard.com> Date: Fri Sep 18 17:16:49 2020 +0300 Update example_upstream_comment locale commit aa9317b0243f5d30f0fcb9cbfcdf502547a8e954 Author: ArtemBaskal <a.baskal@adguard.com> Date: Fri Sep 18 13:28:30 2020 +0300 Highlight comments in custom rules form, extract highlight logic commit dc55245d3db9edbde60fda0a0e50e1e045e71403 Author: ArtemBaskal <a.baskal@adguard.com> Date: Thu Sep 17 22:48:29 2020 +0300 + client: Allow entering comments to the Upstreams box --- client/src/__locales/en.json | 3 +- client/src/actions/index.js | 33 +++- client/src/components/Filters/CustomRules.js | 35 +++- .../Settings/Dns/Upstream/Examples.js | 23 +-- .../components/Settings/Dns/Upstream/Form.js | 166 ++++++++++++------ .../ui/texareaCommentsHighlight.css | 52 ++++++ client/src/helpers/constants.js | 3 + client/src/helpers/form.js | 4 +- client/src/helpers/helpers.js | 14 ++ .../src/helpers/highlightTextareaComments.js | 26 +++ dnsforward/config.go | 1 + dnsforward/dnsforward_http.go | 24 ++- dnsforward/dnsforward_test.go | 26 +-- dnsforward/util.go | 15 ++ home/clients.go | 8 +- 15 files changed, 328 insertions(+), 105 deletions(-) create mode 100644 client/src/components/ui/texareaCommentsHighlight.css create mode 100644 client/src/helpers/highlightTextareaComments.js diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index 550d887e..340c5bcb 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -1,6 +1,7 @@ { "client_settings": "Client settings", "example_upstream_reserved": "You can specify DNS upstream <0>for the specific domain(s)</0>", + "example_upstream_comment": "You can specify the comment", "upstream_parallel": "Use parallel requests to speed up resolving by simultaneously querying all upstream servers", "parallel_requests": "Parallel requests", "load_balancing": "Load-balancing", @@ -581,4 +582,4 @@ "port_53_faq_link": "Port 53 is often occupied by \"DNSStubListener\" or \"systemd-resolved\" services. Please read <0>this instruction</0> on how to resolve this.", "adg_will_drop_dns_queries": "AdGuard Home will be dropping all DNS queries from this client.", "experimental": "Experimental" -} \ No newline at end of file +} diff --git a/client/src/actions/index.js b/client/src/actions/index.js index d39975ea..efd432fd 100644 --- a/client/src/actions/index.js +++ b/client/src/actions/index.js @@ -5,9 +5,15 @@ import axios from 'axios'; import endsWith from 'lodash/endsWith'; import escapeRegExp from 'lodash/escapeRegExp'; import React from 'react'; -import { splitByNewLine, sortClients } from '../helpers/helpers'; +import { compose } from 'redux'; +import { splitByNewLine, sortClients, filterOutComments } from '../helpers/helpers'; import { - BLOCK_ACTIONS, CHECK_TIMEOUT, STATUS_RESPONSE, SETTINGS_NAMES, FORM_NAME, GETTING_STARTED_LINK, + BLOCK_ACTIONS, + CHECK_TIMEOUT, + STATUS_RESPONSE, + SETTINGS_NAMES, + FORM_NAME, + GETTING_STARTED_LINK, } from '../helpers/constants'; import { areEqualVersions } from '../helpers/version'; import { getTlsStatus } from './encryption'; @@ -289,14 +295,21 @@ export const testUpstreamRequest = createAction('TEST_UPSTREAM_REQUEST'); export const testUpstreamFailure = createAction('TEST_UPSTREAM_FAILURE'); export const testUpstreamSuccess = createAction('TEST_UPSTREAM_SUCCESS'); -export const testUpstream = (config) => async (dispatch) => { +export const testUpstream = ( + { bootstrap_dns, upstream_dns }, upstream_dns_file, +) => async (dispatch) => { dispatch(testUpstreamRequest()); try { - const values = { ...config }; - values.bootstrap_dns = splitByNewLine(values.bootstrap_dns); - values.upstream_dns = splitByNewLine(values.upstream_dns); + const removeComments = compose(filterOutComments, splitByNewLine); - const upstreamResponse = await apiClient.testUpstream(values); + const config = { + bootstrap_dns: splitByNewLine(bootstrap_dns), + ...(upstream_dns_file ? null : { + upstream_dns: removeComments(upstream_dns), + }), + }; + + const upstreamResponse = await apiClient.testUpstream(config); const testMessages = Object.keys(upstreamResponse) .map((key) => { const message = upstreamResponse[key]; @@ -317,6 +330,12 @@ export const testUpstream = (config) => async (dispatch) => { } }; +export const testUpstreamWithFormValues = () => async (dispatch, getState) => { + const { upstream_dns_file } = getState().dnsConfig; + const { bootstrap_dns, upstream_dns } = getState().form[FORM_NAME.UPSTREAM].values; + return dispatch(testUpstream({ bootstrap_dns, upstream_dns }, upstream_dns_file)); +}; + export const changeLanguageRequest = createAction('CHANGE_LANGUAGE_REQUEST'); export const changeLanguageFailure = createAction('CHANGE_LANGUAGE_FAILURE'); export const changeLanguageSuccess = createAction('CHANGE_LANGUAGE_SUCCESS'); diff --git a/client/src/components/Filters/CustomRules.js b/client/src/components/Filters/CustomRules.js index 6a423026..723add5c 100644 --- a/client/src/components/Filters/CustomRules.js +++ b/client/src/components/Filters/CustomRules.js @@ -1,13 +1,18 @@ -import React, { Component, Fragment } from 'react'; +import React, { Component } from 'react'; import PropTypes from 'prop-types'; import { Trans, withTranslation } from 'react-i18next'; - +import classnames from 'classnames'; import Card from '../ui/Card'; import PageTitle from '../ui/PageTitle'; import Examples from './Examples'; import Check from './Check'; +import { getTextareaCommentsHighlight, syncScroll } from '../../helpers/highlightTextareaComments'; +import { COMMENT_LINE_DEFAULT_TOKEN, isFirefox } from '../../helpers/constants'; +import '../ui/texareaCommentsHighlight.css'; class CustomRules extends Component { + ref = React.createRef(); + componentDidMount() { this.props.getFilteringStatus(); } @@ -34,6 +39,8 @@ class CustomRules extends Component { this.props.checkHost(values); }; + onScroll = (e) => syncScroll(e, this.ref) + render() { const { t, @@ -47,17 +54,31 @@ class CustomRules extends Component { } = this.props; return ( - <Fragment> + <> <PageTitle title={t('custom_filtering_rules')} /> <Card subtitle={t('custom_filter_rules_hint')} > <form onSubmit={this.handleSubmit}> + <div className={classnames('col-12 text-edit-container form-control--textarea-large', { + 'mb-4': !isFirefox, + 'mb-6': isFirefox, + })}> <textarea - className="form-control form-control--textarea-large font-monospace" - value={userRules} - onChange={this.handleChange} + className={classnames('form-control font-monospace text-input form-control--textarea-large', { + 'text-input--largest': isFirefox, + })} + value={userRules} + onChange={this.handleChange} + onScroll={this.onScroll} /> + {getTextareaCommentsHighlight( + this.ref, + userRules, + classnames({ 'form-control--textarea-large': isFirefox }), + [COMMENT_LINE_DEFAULT_TOKEN, '!'], + )} + </div> <div className="card-actions"> <button className="btn btn-success btn-standard btn-large" @@ -78,7 +99,7 @@ class CustomRules extends Component { onSubmit={this.handleCheck} processing={processingCheck} /> - </Fragment> + </> ); } } diff --git a/client/src/components/Settings/Dns/Upstream/Examples.js b/client/src/components/Settings/Dns/Upstream/Examples.js index 70797909..496fea17 100644 --- a/client/src/components/Settings/Dns/Upstream/Examples.js +++ b/client/src/components/Settings/Dns/Upstream/Examples.js @@ -1,25 +1,10 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Trans, withTranslation } from 'react-i18next'; +import { COMMENT_LINE_DEFAULT_TOKEN } from '../../../../helpers/constants'; const Examples = (props) => ( <div className="list leading-loose"> - <p> - <Trans - components={[ - <a - href="https://kb.adguard.com/general/dns-providers" - target="_blank" - rel="noopener noreferrer" - key="0" - > - DNS providers - </a>, - ]} - > - dns_providers - </Trans> - </p> <Trans>examples_title</Trans>: <ol className="leading-loose"> <li> @@ -141,6 +126,12 @@ const Examples = (props) => ( </Trans> </span> </li> + <li> + <code>{COMMENT_LINE_DEFAULT_TOKEN} comment</code> – + <span> + <Trans>example_upstream_comment</Trans> + </span> + </li> </ol> </div> ); diff --git a/client/src/components/Settings/Dns/Upstream/Form.js b/client/src/components/Settings/Dns/Upstream/Form.js index 1a81d60a..d44a8018 100644 --- a/client/src/components/Settings/Dns/Upstream/Form.js +++ b/client/src/components/Settings/Dns/Upstream/Form.js @@ -1,43 +1,114 @@ -import React from 'react'; +import React, { useRef } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import PropTypes from 'prop-types'; import { Field, reduxForm } from 'redux-form'; import { Trans, useTranslation } from 'react-i18next'; import classnames from 'classnames'; - import Examples from './Examples'; import { renderRadioField, renderTextareaField } from '../../../../helpers/form'; -import { DNS_REQUEST_OPTIONS, FORM_NAME, UPSTREAM_CONFIGURATION_WIKI_LINK } from '../../../../helpers/constants'; -import { testUpstream } from '../../../../actions'; -import { removeEmptyLines } from '../../../../helpers/helpers'; +import { + DNS_REQUEST_OPTIONS, + FORM_NAME, + isFirefox, + UPSTREAM_CONFIGURATION_WIKI_LINK, +} from '../../../../helpers/constants'; +import { testUpstreamWithFormValues } from '../../../../actions'; +import { removeEmptyLines, trimLinesAndRemoveEmpty } from '../../../../helpers/helpers'; +import { getTextareaCommentsHighlight, syncScroll } from '../../../../helpers/highlightTextareaComments'; +import '../../../ui/texareaCommentsHighlight.css'; const UPSTREAM_DNS_NAME = 'upstream_dns'; +const UPSTREAM_MODE_NAME = 'upstream_mode'; -const Title = () => { - const components = { - a: <a href={UPSTREAM_CONFIGURATION_WIKI_LINK} target="_blank" - rel="noopener noreferrer" />, - }; +const renderField = ({ + name, component, type, className, placeholder, + subtitle, value, normalizeOnBlur, containerClass, onScroll, +}) => { + const { t } = useTranslation(); + const processingTestUpstream = useSelector((state) => state.settings.processingTestUpstream); + const processingSetConfig = useSelector((state) => state.dnsConfig.processingSetConfig); - return <label className="form__label" htmlFor={UPSTREAM_DNS_NAME}> - <Trans components={components}>upstream_dns_help</Trans> - </label>; + return <div + key={placeholder} + className={classnames('col-12 mb-4', containerClass)} + > + <Field + id={name} + value={value} + name={name} + component={component} + type={type} + className={className} + placeholder={t(placeholder)} + subtitle={t(subtitle)} + disabled={processingSetConfig || processingTestUpstream} + normalizeOnBlur={normalizeOnBlur} + onScroll={onScroll} + /> + </div>; }; -const getInputFields = (upstream_dns_file) => [ +renderField.propTypes = { + name: PropTypes.string.isRequired, + component: PropTypes.element.isRequired, + type: PropTypes.string.isRequired, + className: PropTypes.string, + placeholder: PropTypes.string.isRequired, + subtitle: PropTypes.string, + value: PropTypes.string, + normalizeOnBlur: PropTypes.func, + containerClass: PropTypes.string, + onScroll: PropTypes.func, +}; + +const renderTextareaWithHighlightField = (props) => { + const upstream_dns = useSelector((store) => store.form[FORM_NAME.UPSTREAM].values.upstream_dns); + const upstream_dns_file = useSelector((state) => state.dnsConfig.upstream_dns_file); + const ref = useRef(null); + + const onScroll = (e) => syncScroll(e, ref); + + return <> + {renderTextareaField({ + ...props, + disabled: !!upstream_dns_file, + onScroll, + normalizeOnBlur: trimLinesAndRemoveEmpty, + })} + {getTextareaCommentsHighlight(ref, upstream_dns)} + </>; +}; + +renderTextareaWithHighlightField.propTypes = { + className: PropTypes.string.isRequired, + disabled: PropTypes.bool, + id: PropTypes.string.isRequired, + input: PropTypes.object, + meta: PropTypes.object, + normalizeOnBlur: PropTypes.func, + onScroll: PropTypes.func, + placeholder: PropTypes.string.isRequired, + subtitle: PropTypes.string.isRequired, + type: PropTypes.string.isRequired, +}; + +const INPUT_FIELDS = [ { - getTitle: Title, name: UPSTREAM_DNS_NAME, type: 'text', - value: 'test', - component: renderTextareaField, - className: 'form-control form-control--textarea font-monospace', + component: renderTextareaWithHighlightField, + className: classnames('form-control form-control--textarea font-monospace text-input', { + 'text-input--larger': isFirefox, + }), + containerClass: classnames('text-edit-container', { + 'mb-4': !isFirefox, + 'mb-6': isFirefox, + }), placeholder: 'upstream_dns', normalizeOnBlur: removeEmptyLines, - disabled: !!upstream_dns_file, }, { - name: 'upstream_mode', + name: UPSTREAM_MODE_NAME, type: 'radio', value: DNS_REQUEST_OPTIONS.LOAD_BALANCING, component: renderRadioField, @@ -45,7 +116,7 @@ const getInputFields = (upstream_dns_file) => [ placeholder: 'load_balancing', }, { - name: 'upstream_mode', + name: UPSTREAM_MODE_NAME, type: 'radio', value: DNS_REQUEST_OPTIONS.PARALLEL, component: renderRadioField, @@ -53,7 +124,7 @@ const getInputFields = (upstream_dns_file) => [ placeholder: 'parallel_requests', }, { - name: 'upstream_mode', + name: UPSTREAM_MODE_NAME, type: 'radio', value: DNS_REQUEST_OPTIONS.FASTEST_ADDR, component: renderRadioField, @@ -68,44 +139,39 @@ const Form = ({ const dispatch = useDispatch(); const { t } = useTranslation(); const upstream_dns = useSelector((store) => store.form[FORM_NAME.UPSTREAM].values.upstream_dns); - const bootstrap_dns = useSelector( - (store) => store.form[FORM_NAME.UPSTREAM].values.bootstrap_dns, - ); - const upstream_dns_file = useSelector((state) => state.dnsConfig.upstream_dns_file); const processingTestUpstream = useSelector((state) => state.settings.processingTestUpstream); const processingSetConfig = useSelector((state) => state.dnsConfig.processingSetConfig); - const handleUpstreamTest = () => dispatch(testUpstream({ - upstream_dns, - bootstrap_dns, - })); + const handleUpstreamTest = () => dispatch(testUpstreamWithFormValues()); const testButtonClass = classnames('btn btn-primary btn-standard mr-2', { 'btn-loading': processingTestUpstream, }); - const INPUT_FIELDS = getInputFields(upstream_dns_file); + const components = { + a: <a href={UPSTREAM_CONFIGURATION_WIKI_LINK} target="_blank" + rel="noopener noreferrer" />, + }; - return <form onSubmit={handleSubmit}> + return <form onSubmit={handleSubmit} className="form--upstream"> <div className="row"> - {INPUT_FIELDS.map(({ - name, component, type, className, placeholder, - getTitle, subtitle, disabled, value, normalizeOnBlur, - }) => <div className="col-12 mb-4" key={placeholder}> - {typeof getTitle === 'function' && getTitle()} - <Field - id={name} - value={value} - name={name} - component={component} - type={type} - className={className} - placeholder={t(placeholder)} - subtitle={t(subtitle)} - disabled={processingSetConfig || processingTestUpstream || disabled} - normalizeOnBlur={normalizeOnBlur} - /> - </div>)} + <label className="col form__label" htmlFor={UPSTREAM_DNS_NAME}> + <Trans components={components}>upstream_dns_help</Trans> + {' '} + <Trans components={[ + <a + href="https://kb.adguard.com/general/dns-providers" + target="_blank" + rel="noopener noreferrer" + key="0" + > + DNS providers + </a>, + ]}> + dns_providers + </Trans> + </label> + {INPUT_FIELDS.map(renderField)} <div className="col-12"> <Examples /> <hr /> diff --git a/client/src/components/ui/texareaCommentsHighlight.css b/client/src/components/ui/texareaCommentsHighlight.css new file mode 100644 index 00000000..8551966d --- /dev/null +++ b/client/src/components/ui/texareaCommentsHighlight.css @@ -0,0 +1,52 @@ +.text-edit-container { + position: relative; + height: 10rem; +} + +.text-input, +.text-output { + position: absolute; + top: 0; + left: 0; + width: 100%; + height: 100%; + padding: 1rem; + background: transparent; + white-space: pre-wrap; + line-height: 1.5rem; + word-wrap: break-word; + font-size: 0.9375rem; + margin: 0; +} + +.form--upstream .text-input, +.form--upstream .text-output { + width: 98%; + left: 1%; +} + +.text-input { + opacity: 1; + resize: none; + height: 10rem; +} + +.text-input--larger { + height: 11rem; +} + +.text-input--largest { + height: 16rem; +} + +.text-output { + pointer-events: none; + z-index: 3; + overflow-y: auto; + background: transparent; + border: 1px solid transparent; +} + +.text-transparent { + color: transparent; +} diff --git a/client/src/helpers/constants.js b/client/src/helpers/constants.js index 2ab176ea..91ec294e 100644 --- a/client/src/helpers/constants.js +++ b/client/src/helpers/constants.js @@ -568,3 +568,6 @@ export const CACHE_CONFIG_FIELDS = { cache_ttl_min: 'cache_ttl_min', cache_ttl_max: 'cache_ttl_max', }; + +export const isFirefox = navigator.userAgent.indexOf('Firefox') !== -1; +export const COMMENT_LINE_DEFAULT_TOKEN = '#'; diff --git a/client/src/helpers/form.js b/client/src/helpers/form.js index f26c64a5..3b6801a0 100644 --- a/client/src/helpers/form.js +++ b/client/src/helpers/form.js @@ -7,7 +7,7 @@ import { R_UNIX_ABSOLUTE_PATH, R_WIN_ABSOLUTE_PATH } from './constants'; export const renderField = (props, elementType) => { const { - input, id, className, placeholder, type, disabled, normalizeOnBlur, + input, id, className, placeholder, type, disabled, normalizeOnBlur, onScroll, autoComplete, meta: { touched, error }, min, max, step, } = props; @@ -25,6 +25,7 @@ export const renderField = (props, elementType) => { max, step, onBlur, + onScroll, }); return ( @@ -48,6 +49,7 @@ renderField.propTypes = { min: PropTypes.number, max: PropTypes.number, step: PropTypes.number, + onScroll: PropTypes.func, meta: PropTypes.shape({ touched: PropTypes.bool, error: PropTypes.string, diff --git a/client/src/helpers/helpers.js b/client/src/helpers/helpers.js index 27177861..8cdb5384 100644 --- a/client/src/helpers/helpers.js +++ b/client/src/helpers/helpers.js @@ -16,6 +16,7 @@ import { getTrackerData } from './trackers/trackers'; import { ADDRESS_TYPES, CHECK_TIMEOUT, + COMMENT_LINE_DEFAULT_TOKEN, CUSTOM_FILTERING_RULES_ID, DEFAULT_DATE_FORMAT_OPTIONS, DEFAULT_LANGUAGE, @@ -316,6 +317,12 @@ export const trimMultilineString = (text) => splitByNewLine(text) export const removeEmptyLines = (text) => splitByNewLine(text) .join('\n'); +/** + * @param {string} input + * @returns {string} + */ +export const trimLinesAndRemoveEmpty = (input) => input.split('\n').map((line) => line.trim()).filter(Boolean).join('\n'); + /** * Normalizes the topClients array * @@ -930,3 +937,10 @@ export const getBlockingClientName = (clients, ip) => { } return ip; }; + +/** + * @param {string[]} lines + * @returns {string[]} + */ +export const filterOutComments = (lines) => lines + .filter((line) => !line.startsWith(COMMENT_LINE_DEFAULT_TOKEN)); diff --git a/client/src/helpers/highlightTextareaComments.js b/client/src/helpers/highlightTextareaComments.js new file mode 100644 index 00000000..ad053246 --- /dev/null +++ b/client/src/helpers/highlightTextareaComments.js @@ -0,0 +1,26 @@ +import React from 'react'; +import classnames from 'classnames'; +import { COMMENT_LINE_DEFAULT_TOKEN } from './constants'; + +const renderHighlightedLine = (line, idx, commentLineTokens = [COMMENT_LINE_DEFAULT_TOKEN]) => { + const isComment = commentLineTokens.some((token) => line.trim().startsWith(token)); + + const lineClassName = classnames({ + 'text-gray': isComment, + 'text-transparent': !isComment, + }); + + return <div className={lineClassName} key={idx}>{line || '\n'}</div>; +}; +export const getTextareaCommentsHighlight = ( + ref, lines, className = '', commentLineTokens, +) => { + const renderLine = (line, idx) => renderHighlightedLine(line, idx, commentLineTokens); + + return <code className={classnames('text-output', className)} ref={ref}>{lines?.split('\n').map(renderLine)}</code>; +}; + +export const syncScroll = (e, ref) => { + // eslint-disable-next-line no-param-reassign + ref.current.scrollTop = e.target.scrollTop; +}; diff --git a/dnsforward/config.go b/dnsforward/config.go index 8d27d1ff..cd245ee2 100644 --- a/dnsforward/config.go +++ b/dnsforward/config.go @@ -249,6 +249,7 @@ func (s *Server) prepareUpstreamSettings() error { } else { upstreams = s.conf.UpstreamDNS } + upstreams = filterOutComments(upstreams) upstreamConfig, err := proxy.ParseUpstreamsConfig(upstreams, s.conf.BootstrapDNS, DefaultTimeout) if err != nil { return fmt.Errorf("DNS: proxy.ParseUpstreamsConfig: %s", err) diff --git a/dnsforward/dnsforward_http.go b/dnsforward/dnsforward_http.go index 42c738f2..480f977e 100644 --- a/dnsforward/dnsforward_http.go +++ b/dnsforward/dnsforward_http.go @@ -117,12 +117,10 @@ func (s *Server) handleSetConfig(w http.ResponseWriter, r *http.Request) { } if js.Exists("upstream_dns") { - if len(req.Upstreams) != 0 { - err = ValidateUpstreams(req.Upstreams) - if err != nil { - httpError(r, w, http.StatusBadRequest, "wrong upstreams specification: %s", err) - return - } + err = ValidateUpstreams(req.Upstreams) + if err != nil { + httpError(r, w, http.StatusBadRequest, "wrong upstreams specification: %s", err) + return } } @@ -256,6 +254,14 @@ type upstreamJSON struct { // ValidateUpstreams validates each upstream and returns an error if any upstream is invalid or if there are no default upstreams specified func ValidateUpstreams(upstreams []string) error { + // No need to validate comments + upstreams = filterOutComments(upstreams) + + // Consider this case valid because defaultDNS will be used + if len(upstreams) == 0 { + return nil + } + var defaultUpstreamFound bool for _, u := range upstreams { d, err := validateUpstream(u) @@ -397,6 +403,10 @@ func (s *Server) handleTestUpstreamDNS(w http.ResponseWriter, r *http.Request) { } func checkDNS(input string, bootstrap []string) error { + if !isUpstream(input) { + return nil + } + // separate upstream from domains list input, defaultUpstream, err := separateUpstream(input) if err != nil { @@ -404,7 +414,7 @@ func checkDNS(input string, bootstrap []string) error { } // No need to check this DNS server - if input == "#" || !defaultUpstream { + if !defaultUpstream { return nil } diff --git a/dnsforward/dnsforward_test.go b/dnsforward/dnsforward_test.go index 2fb89075..889321b1 100644 --- a/dnsforward/dnsforward_test.go +++ b/dnsforward/dnsforward_test.go @@ -1005,31 +1005,35 @@ func TestValidateUpstream(t *testing.T) { } func TestValidateUpstreamsSet(t *testing.T) { + // Empty upstreams array + var upstreamsSet []string + err := ValidateUpstreams(upstreamsSet) + assert.Nil(t, err, "empty upstreams array should be valid") + + // Comment in upstreams array + upstreamsSet = []string{"# comment"} + err = ValidateUpstreams(upstreamsSet) + assert.Nil(t, err, "comments should not be validated") + // Set of valid upstreams. There is no default upstream specified - upstreamsSet := []string{"[/host.com/]1.1.1.1", + upstreamsSet = []string{"[/host.com/]1.1.1.1", "[//]tls://1.1.1.1", "[/www.host.com/]#", "[/host.com/google.com/]8.8.8.8", "[/host/]sdns://AQMAAAAAAAAAFDE3Ni4xMDMuMTMwLjEzMDo1NDQzINErR_JS3PLCu_iZEIbq95zkSV2LFsigxDIuUso_OQhzIjIuZG5zY3J5cHQuZGVmYXVsdC5uczEuYWRndWFyZC5jb20", } - err := ValidateUpstreams(upstreamsSet) - if err == nil { - t.Fatalf("there is no default upstream") - } + err = ValidateUpstreams(upstreamsSet) + assert.NotNil(t, err, "there is no default upstream") // Let's add default upstream upstreamsSet = append(upstreamsSet, "8.8.8.8") err = ValidateUpstreams(upstreamsSet) - if err != nil { - t.Fatalf("upstreams set is valid, but doesn't pass through validation cause: %s", err) - } + assert.Nilf(t, err, "upstreams set is valid, but doesn't pass through validation cause: %s", err) // Let's add invalid upstream upstreamsSet = append(upstreamsSet, "dhcp://fake.dns") err = ValidateUpstreams(upstreamsSet) - if err == nil { - t.Fatalf("there is an invalid upstream in set, but it pass through validation") - } + assert.NotNil(t, err, "there is an invalid upstream in set, but it pass through validation") } func TestIpFromAddr(t *testing.T) { diff --git a/dnsforward/util.go b/dnsforward/util.go index f159f43a..be8f23b7 100644 --- a/dnsforward/util.go +++ b/dnsforward/util.go @@ -85,3 +85,18 @@ func matchDNSName(dnsNames []string, sni string) bool { } return false } + +// Is not comment +func isUpstream(line string) bool { + return !strings.HasPrefix(line, "#") +} + +func filterOutComments(lines []string) []string { + var filtered []string + for _, l := range lines { + if isUpstream(l) { + filtered = append(filtered, l) + } + } + return filtered +} diff --git a/home/clients.go b/home/clients.go index 1e6ae413..252c6e2b 100644 --- a/home/clients.go +++ b/home/clients.go @@ -404,11 +404,9 @@ func (clients *clientsContainer) check(c *Client) error { } sort.Strings(c.Tags) - if len(c.Upstreams) != 0 { - err := dnsforward.ValidateUpstreams(c.Upstreams) - if err != nil { - return fmt.Errorf("invalid upstream servers: %s", err) - } + err := dnsforward.ValidateUpstreams(c.Upstreams) + if err != nil { + return fmt.Errorf("invalid upstream servers: %s", err) } return nil