+ 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

@@ -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"
-}
+}

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 };
+        const removeComments = compose(filterOutComments, splitByNewLine);
-        values.bootstrap_dns = splitByNewLine(values.bootstrap_dns);
-        values.upstream_dns = splitByNewLine(values.upstream_dns);
 
-        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');

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"
+                                className={classnames('form-control font-monospace text-input form-control--textarea-large', {
-                            value={userRules}
+                                    'text-input--largest': isFirefox,
-                            onChange={this.handleChange}
+                                })}
+                                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>
+            </>
         );
     }
 }

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> –&nbsp;
+                <span>
+                    <Trans>example_upstream_comment</Trans>
+                </span>
+            </li>
         </ol>
     </div>
 );

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 {
-import { testUpstream } from '../../../../actions';
+    DNS_REQUEST_OPTIONS,
-import { removeEmptyLines } from '../../../../helpers/helpers';
+    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 renderField = ({
-    const components = {
+    name, component, type, className, placeholder,
-        a: <a href={UPSTREAM_CONFIGURATION_WIKI_LINK} target="_blank"
+    subtitle, value, normalizeOnBlur, containerClass, onScroll,
-              rel="noopener noreferrer" />,
+}) => {
-    };
+    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}>
+    return <div
-        <Trans components={components}>upstream_dns_help</Trans>
+            key={placeholder}
-    </label>;
+            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: renderTextareaWithHighlightField,
-        component: renderTextareaField,
+        className: classnames('form-control form-control--textarea font-monospace text-input', {
-        className: 'form-control form-control--textarea font-monospace',
+            '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({
+    const handleUpstreamTest = () => dispatch(testUpstreamWithFormValues());
-        upstream_dns,
-        bootstrap_dns,
-    }));
 
     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(({
+            <label className="col form__label" htmlFor={UPSTREAM_DNS_NAME}>
-                name, component, type, className, placeholder,
+                <Trans components={components}>upstream_dns_help</Trans>
-                getTitle, subtitle, disabled, value, normalizeOnBlur,
+                {' '}
-            }) => <div className="col-12 mb-4" key={placeholder}>
+                <Trans components={[
-                {typeof getTitle === 'function' && getTitle()}
+                    <a
-                <Field
+                            href="https://kb.adguard.com/general/dns-providers"
-                    id={name}
+                            target="_blank"
-                    value={value}
+                            rel="noopener noreferrer"
-                    name={name}
+                            key="0"
-                    component={component}
+                    >
-                    type={type}
+                        DNS providers
-                    className={className}
+                    </a>,
-                    placeholder={t(placeholder)}
+                ]}>
-                    subtitle={t(subtitle)}
+                    dns_providers
-                    disabled={processingSetConfig || processingTestUpstream || disabled}
+                </Trans>
-                    normalizeOnBlur={normalizeOnBlur}
+            </label>
-                />
+            {INPUT_FIELDS.map(renderField)}
-            </div>)}
             <div className="col-12">
                 <Examples />
                 <hr />

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;
+}

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 = '#';

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,

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));

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;
+};

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)

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)
-			err = ValidateUpstreams(req.Upstreams)
+		if err != nil {
-			if err != nil {
+			httpError(r, w, http.StatusBadRequest, "wrong upstreams specification: %s", err)
-				httpError(r, w, http.StatusBadRequest, "wrong upstreams specification: %s", err)
+			return
-				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
 	}
 

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)
+	err = ValidateUpstreams(upstreamsSet)
-	if err == nil {
+	assert.NotNil(t, err, "there is no default upstream")
-		t.Fatalf("there is no default upstream")
-	}
 
 	// Let's add default upstream
 	upstreamsSet = append(upstreamsSet, "8.8.8.8")
 	err = ValidateUpstreams(upstreamsSet)
-	if err != nil {
+	assert.Nilf(t, err, "upstreams set is valid, but doesn't pass through validation cause: %s", err)
-		t.Fatalf("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 {
+	assert.NotNil(t, err, "there is an invalid upstream in set, but it pass through validation")
-		t.Fatalf("there is an invalid upstream in set, but it pass through validation")
-	}
 }
 
 func TestIpFromAddr(t *testing.T) {

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
+}

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)
-		err := dnsforward.ValidateUpstreams(c.Upstreams)
+	if err != nil {
-		if err != nil {
+		return fmt.Errorf("invalid upstream servers: %s", err)
-			return fmt.Errorf("invalid upstream servers: %s", err)
-		}
 	}
 
 	return nil